Zad5 #125
Zad5 #125
Conversation
Updating fork
homework_4
merged new homework
|
Znalazlem sposób jak zachowywac spojnosc siege ability po zmianie pola warriora na inny building |
|
Chyba rspec nie wywoluje te callbacks na before_update i after_update |
| @@ -1,7 +1,21 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| class BuildingsController < ApplicationController | |||
| def index; end | |||
| def index | |||
| render json: Building.all.order(:id), include: [:warriors] | |||
There was a problem hiding this comment.
Id nie jest domyslnym orderem? :D
| end | ||
| def self.call(building:) | ||
| # this should stop the method if getting null | ||
| return unless building |
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 :)
| if building.warriors.any? | ||
| warriors_count = building.warriors.where(type: 'Warriors::Samurai').count | ||
| hussar_count = building.warriors.where(type: 'Warriors::Hussar').count | ||
| total_rice_need = warriors_count + hussar_count * 2 + building.default_rice_need |
There was a problem hiding this comment.
Te powyższe trzy linijki ładniej by wyglądałī, jeśli Warrior miałby zaimplementowaną metodę/atrybut np. supply_burn_rate, który zwraca 1 jeśli warrior nie ma konia i 2 jeśli go ma, wtedy nie trzeba by myśleć nad takimi ifami w tym serwisie :)
| # if warrior is changing his buildings | ||
| if building_id_was != building_id | ||
| # save this attribute wihtout triggering callbacks | ||
| update_column(:prev_building_id, building_id_was) |
There was a problem hiding this comment.
Z tego co pamiętam update_column omija walidację więc nie jest to najlepszy sposób na edycje kolumny :D Najprosciej byłoby dodać relację typu belongs_to z opcją optional: true :) Optional pozwoli na ustawianie nila na tej kolumnie, nie trzeba nic zmieniać w bazie, bo bazy domyślnie pozwalają na nile, a jedynie rails sam waliduje to że nie mogą być nilem :)
There was a problem hiding this comment.
Poza tym w sumie gdy sie wbijasz callbackiem miedzy zmiane danych a sve, masz dostep do zmiennej metody changes, ktora zwraca to co sie zmienilo, dzieki czemu nie ma potrzeby w tym przypadku poprzednim budynek na uzytkowniku, bo w callbacku informacja z jakiego na jakiego zmienilo sie building id juz jest
|
|
||
| expect(building.siege_ability).to eq(250) | ||
| end | ||
| # Fail got 500 instead of 0 |
There was a problem hiding this comment.
To może wynikać z tego, że dla building trzeba przeładować atrybuty i asocjacje z bazy za pomocą metody '.reload'
Wyliczanie wartosci siege ability jest prowadzone po kazdym create, update, destroy warriorów ktore nalezą do tego building => to nie powino obciązac baze