Good stuff getting to this challenge! Here are my notes on your code:
-
It looks like you have methods on your specific tree classes that are similar. For example,
OrangeTree.has_oranges?
andPearTree.has_pears?
. This is a code smell. You could DRY out your code by putting ahas_fruit?
method on theFruitTree
class. This way, every child class ofFruitTree
will have ahas_fruit?
method. Tada! Can you find any other ways to DRY out your code? Hint: look for similarly-worded methods that appear in all of your tree classes. -
Each of your tree classes has a few lines of code in its age! method that needs to be rethought. Here are those lines, from the
AppleTree
class:if age > 3 && age < 17 new_number = rand(10..25) number = (1..new_number).to_a number.map {|apple| @fruit << apple = Apple.new(rand(1..4))} end
I understand what you want to do here (create a random number of
Apple
objects and encapsulate them into yourAppleTree
object) but your code as it is written is unnecessarily doing much more than this. Remember:.map
takes an existing array and creates & returns a new array. You don't want to do that here, so.map
is not a good choice.How about something like this:
if age > 3 && age < 17 rand(10..25).times do @fruit << Apple.new(rand(1..4)) end end
Notice how the
times
method gives Ruby the ability to loop a specified number of times, and also how.times
can be chanined onto therand
method's return value, to loop a random number of times. -
Your code is tracking the life status of your trees with a @life instance variable that holds a string, either
"alive"
or"dead"
. This is a code smell also. Any attribute that is either "on" or "off" can often be handled better with a boolean. Like this:@alive = true
or the other way:
@dead = false
With a boolean value, your
dead?
method now becomes a lot easier to write:def dead? @dead # or !@alive if you use @alive end
If you have any questions let me know. I hope this helps!
-Phil