Describe an apple tree that ages, grows, etc#59
Describe an apple tree that ages, grows, etc#59RupertSaxton wants to merge 5 commits intopaircolumbus:masterfrom
Conversation
|
Sorry that I didn't make it into smaller commits |
lib/tree.rb
Outdated
| @age = 0 | ||
| @apples = [] | ||
| @alive = true | ||
| @colors = ['red', 'green', 'yellow'] |
There was a problem hiding this comment.
Should apple colors be a property of the tree, or of the apple? I guess you could make a case either way. But given what you're doing with it here, it might make sense to move the random color- and size-generation into Apple#initialize
lib/tree.rb
Outdated
|
|
||
| def add_apples | ||
| rand(1..(4*@age)).times do | ||
| @apples << Apple.new(@colors[rand(0..2)], rand(1..5)) |
lib/tree.rb
Outdated
| end | ||
|
|
||
| def any_apples? | ||
| @apples.length > 0 ? true : false |
There was a problem hiding this comment.
There is empty?, and at least when you have ActiveSupport available, present?.
lib/tree.rb
Outdated
| true | ||
| else | ||
| false | ||
| end |
There was a problem hiding this comment.
This could just be @height > 60 || @age > 13, right?
But what I'm really wondering is why it isn't just !@alive
There was a problem hiding this comment.
I like the simplicity of @height > 60 || @age > 13. Since @alive isn't used or necessary I think I'll just remove it
lib/tree.rb
Outdated
| end | ||
|
|
||
| avg_diameter = # It's up to you to calculate the average diameter for this harvest. | ||
| avg_diameter = diameter_sum/basket.length |
There was a problem hiding this comment.
This looks like it will always return an integer. Is that what is intended?
| it 'will age' do | ||
| tree.age! | ||
| expect(tree.age).to eq 1 | ||
| end |
There was a problem hiding this comment.
One option here is to set the age to a random number, then make it age, then check that the new age is one higher than the old one.
|
|
||
| it 'raises an error for no apples' do | ||
| expect { tree.pick_an_apple! }.to raise_error(NoApplesError, "This tree has no apples") | ||
| end |
spec/tree_spec.rb
Outdated
| end | ||
|
|
||
| it 'removes an apple when present' do | ||
| tree.age! |
There was a problem hiding this comment.
add_apples might be clearer setup here
spec/tree_spec.rb
Outdated
| tree.age! | ||
| remaining_apples = tree.apples.length - 1 | ||
| tree.pick_an_apple! | ||
| expect(tree.apples.length).to eq remaining_apples |
There was a problem hiding this comment.
remaining_apples makes it sounds like the value is an array of apples. Maybe remaining_apple_count or similar.
2 participants
No description provided.