Skip to content

Zadanie 5#129

Open
StachuraMichal wants to merge 3 commits intoDaftAcademy:masterfrom
StachuraMichal:zad5
Open

Zadanie 5#129
StachuraMichal wants to merge 3 commits intoDaftAcademy:masterfrom
StachuraMichal:zad5

Conversation

@StachuraMichal
Copy link
Copy Markdown

No description provided.


consume_rate = 10
@building.warriors.each do |warrior|
consume_rate += if warrior.type == 'Warriors::Hussar'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ładniej by to chyba wyglądało, 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 :)


def call
raise NotImprementedYet
return 0 if @building.warriors.all.empty?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

factory :warrior, class: 'Warrior' do
association(:building)
association(:clan)
name { 'default' }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tu powinienes zastosowac onstrukcje w tym stylu zeby fabryka byla dobra, bo z tego co pamietam warrior ma unikalne imie wsrod tych zyjacych :)

 sequence :email do |n|
    "person#{n}@example.com"
  end

let(:building_granary) { 100 }

it 'responds with 200' do
get "/buildings/#{building.id}"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

W tym describe, tego get mozna bylo wyneisc do before

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants