Conversation
|
|
||
| def call | ||
| raise NotImprementedYet | ||
| siege = @warriors.present? ? @building.siege_ability = granary_ability : @building.siege_ability = 0 |
There was a problem hiding this comment.
To sprawdzenie spokojnie by się mogło znaleźć gdzieś "wyżej", żeby nie obciążać logiki serwisu :) Serwis najlepiej jeśli ma jedną odpowiedzialność, a Twój serwis poza liczeniem ile dni przetrwa dany budynek, stwierdza upadłość budynku jeśli nie ma w nim wojowników :)
| end | ||
| end | ||
|
|
||
| describe '#call' do |
There was a problem hiding this comment.
jeden describe wrapujący całość dotyczących go testów by wystarczył :)
| def call | ||
| raise NotImprementedYet | ||
| siege = @warriors.present? ? @building.siege_ability = granary_ability : @building.siege_ability = 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
There was a problem hiding this comment.
Hej,
dziękuję za komentarze, postaram się nanieść poprawki po majówce;-)
Pozdrawiam
2 participants
No description provided.