Sheshtawy #74
Sheshtawy #74
Conversation
lib/tree.rb
Outdated
| @@ -1,38 +1,57 @@ | |||
| class NoApplesError < StandardError; end | |||
|
|
|||
| class Tree | |||
| attr_#fill_in :height, :age, :apples, :alive | |||
| attr_accessor :height, :age, :apples, :alive | |||
There was a problem hiding this comment.
should all attributes be settable?
| end | ||
|
|
||
| def age! | ||
| def age! # I'm not sure why this has a bang |
There was a problem hiding this comment.
lib/tree.rb
Outdated
| end | ||
|
|
||
| def pick_an_apple! | ||
| raise NoApplesError, "This tree has no apples" unless self.any_apples? | ||
| if self.any_apples? | ||
| return @apples.pop |
There was a problem hiding this comment.
| @@ -2,13 +2,93 @@ | |||
| require_relative '../lib/tree' | |||
|
|
|||
| describe Tree do | |||
| let(:tree) { Tree.new(1, [], true, 1) } | |||
There was a problem hiding this comment.
nice use of .let. in rails apps, you'll often see use of a library like factory_bot for building factories to use in test. there are also fixture libraries that are popular.
spec/tree_spec.rb
Outdated
| expect(tree.dead?).to be true | ||
| end | ||
|
|
||
| it 'is alive when calling dead? returns false' do |
There was a problem hiding this comment.
looks great but i'd recommend using contexts here to have a clear happy vs unhappy path. it also improves readability. contexts can also be nested.
describe Tree do
it 'has a particular attribute' do
...
end
context 'when alive' do
it '..' do
end
end
context 'when dead' do
it '..' do
end
end
end
lib/tree.rb
Outdated
| if @age == 50 | ||
| @alive = false | ||
| elsif | ||
| @age = @age + 1 |
There was a problem hiding this comment.
@age += 1 is the shorthand equivalent.
BTW, I appreciate that it doesn't continue to age after it has died. 👍
lib/tree.rb
Outdated
| end | ||
|
|
||
| def any_apples? | ||
| @apples.length >= 1 ? true : false |
There was a problem hiding this comment.
The ternary here is redundant. @apples.length >= 1 already returns true or false.
@apples.any? should do the trick, too.
Update tests
Remove unnecessary ternary operator
@jaybobo This test suite might not be covering all cases, but I wanted to get your feedback on how I'm writing tests vs best practices, and checking if I'm missing something major