Zadanie5 #137
Zadanie5 #137
Conversation
| @@ -1,7 +1,21 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| class BuildingsController < ApplicationController | |||
| def index; end | |||
| def index | |||
| render json: Building.all.includes( warriors: [:weapon, :clan]), include: :warriors | |||
There was a problem hiding this comment.
tego nie jestem totalnie pewien, patrzyłem w logi serwera i liczba zapytań sql spadła do 4 selectow, natomiast wydajnościowo przy takim małym zakresie danych nie było wielkich różnic. gem bullet to the rescue?
| end | ||
|
|
||
| def call | ||
| raise NotImprementedYet | ||
| @building.siege_ability = (@warriors.present? ? @building.granary / food_demand : 0) |
There was a problem hiding this comment.
i mam pytanie co do wymagan: twierdza potrzebuje też ludzi, którzy nie są wojownikami.
Czy to oznaczało, że powinienem stworzyć dodatkowy model np kucharz i uwzględnić go?
There was a problem hiding this comment.
To oznaczało, że twierdza ma cywili w środku, którzy sprawiają, że codziennie znika 10 jednostek jedzenia. Nie ma potrzeby odwzorowywania cywili jako modeli, tylko że trzeba uwzględnić przy liczeniu ile dni przetrwa twierdza, że chcąc, nie chcąc, codziennie znika 10 jednostek pożywienia
There was a problem hiding this comment.
Poza tym ten warunek ternarny można by przenieść "wyżej" poza serwis do twoich callbacków na userze, bo nieco komplikuje nam logikę i w przyszlości np. po ulepszeniu fortyfikacji fortecy może okazać się, że twierdza jest w stanie przetrwać bez wojowników, a taka zmiana ułatwi rozwój tej klasy
|
|
||
| def food_demand | ||
| daily_basic = 10 | ||
| samurais = @warriors.where(type: 'Warriors::Samurai').count |
There was a problem hiding this comment.
czy tu size sprawdziłby się lepiej?
There was a problem hiding this comment.
Nie bardzo, przez to where może tu wystąpić problem n+1 :) Ale jeśli zamiast where użylibyśmy select'a to problem n+1 z bazy danych został by przeniesiony na złożoność pamięciową naszego serwisu
| end | ||
|
|
||
| def call | ||
| raise NotImprementedYet | ||
| @building.siege_ability = (@warriors.present? ? @building.granary / food_demand : 0) |
There was a problem hiding this comment.
To oznaczało, że twierdza ma cywili w środku, którzy sprawiają, że codziennie znika 10 jednostek jedzenia. Nie ma potrzeby odwzorowywania cywili jako modeli, tylko że trzeba uwzględnić przy liczeniu ile dni przetrwa twierdza, że chcąc, nie chcąc, codziennie znika 10 jednostek pożywienia
|
|
||
| def food_demand | ||
| daily_basic = 10 | ||
| samurais = @warriors.where(type: 'Warriors::Samurai').count |
There was a problem hiding this comment.
Nie bardzo, przez to where może tu wystąpić problem n+1 :) Ale jeśli zamiast where użylibyśmy select'a to problem n+1 z bazy danych został by przeniesiony na złożoność pamięciową naszego serwisu
| end | ||
|
|
||
| def call | ||
| raise NotImprementedYet | ||
| @building.siege_ability = (@warriors.present? ? @building.granary / food_demand : 0) |
There was a problem hiding this comment.
Poza tym ten warunek ternarny można by przenieść "wyżej" poza serwis do twoich callbacków na userze, bo nieco komplikuje nam logikę i w przyszlości np. po ulepszeniu fortyfikacji fortecy może okazać się, że twierdza jest w stanie przetrwać bez wojowników, a taka zmiana ułatwi rozwój tej klasy
| end | ||
|
|
||
| def call | ||
| raise NotImprementedYet | ||
| @building.siege_ability = (@warriors.present? ? @building.granary / food_demand : 0) | ||
| @building.save |
There was a problem hiding this comment.
To zapisywanie atrybutu niepotrzebnie dociąża nam ten serwis, powinno też się znaleźć "wyżej" :) Wiąże się to też między innymi ze wzorcem dobrego projektowania aplikacji, Single Responsibility Principle. Ogólnie możnaby zoptymalizować nasz callback przez obsługę wyliczenia tych dni i zapisania tej wartości na budynku w kolejce asynchronicznej np. Sidekiq, Resque, ActiveJob :D Albo przez w ogóle zastanowieniem się czy nie wystarczy liczyć tych dni w serializerze, który by potrafił scachować wywołanie tego serwisu na jakiś czas, żeby oszczędzić pracy apce, ale to są tematy których nie było na LevelUp, nie mniej są bardzo ważne i polecam je zbadać :) napewno pomyślimy o nich w kolejnych edycjach, a napewno będą na praktykach :D
sorki za wszystkie zmiany... chyba nie umiem w te forki za mocno :(