Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

finish tree #55

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

victoria-hall1998
Copy link

@victoria-hall1998 victoria-hall1998 commented Jun 27, 2017

lib/tree.rb Outdated
class AppleTree
attr_#fill_in :height, :age, :apples, :alive
class Tree
attr_accessor :height, :age, :apples, :alive
Copy link
Member

Choose a reason for hiding this comment

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

why attr_accessor here instead of attr_reader or attr_writer?

lib/tree.rb Outdated
end

def any_apples?
return @apples.length > 0
Copy link
Member

Choose a reason for hiding this comment

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

lib/tree.rb Outdated
end

def add_apples
apple = 12.times { @apples.push Apple.new("Red", Random.rand(1..6)) } #adds between 1-3 more apples every year, with random diameter
Copy link
Member

Choose a reason for hiding this comment

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

what's being done with the apple variable?

describe 'Tree' do
it 'should be a Class' do
expect(described_class.is_a? 'Class').to be_true
expect(Tree_class.is_a? 'Class').to be_true
Copy link
Member

Choose a reason for hiding this comment

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

Do your tests pass when you run rspec?

https://ruby-doc.org/core-2.4.1/Object.html#method-i-is_a-3F

expect(Tree_class.is_a? 'Class').to be_true
end

it 'grows one foot' do
Copy link
Member

Choose a reason for hiding this comment

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

why might grouping your tests into contexts be a good idea?
www.betterspecs.org/#contexts

end

it 'is a Fruit' do
expect(Apple < Fruit).to be true
Copy link
Member

Choose a reason for hiding this comment

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

what's happening here?

Copy link
Member

@jaybobo jaybobo left a comment

Choose a reason for hiding this comment

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

Take another look at betterspecs.org to see how contexts are used. Remember to run your rspec tests to ensure they run.

end

it 'is a Fruit' do
expect(Apple < Fruit).to be true # expects to be inheriting from apple if it is a fruit
Copy link
Member

Choose a reason for hiding this comment

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

I like this. There's a couple of other ways to do this as well.
https://stackoverflow.com/questions/23027236/how-to-test-ruby-inheritance-with-rspec

The documentation spells out your method here.
https://ruby-doc.org/core-2.3.0/Module.html#method-i-3C

end

describe 'Apple' do
apple = Apple.new("Red", 30)
Copy link
Member

Choose a reason for hiding this comment

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

For things like this, we use let. Refactor any variables used here using let instead.
http://www.betterspecs.org/#let


context 'when dies after 25 years' do
it "equals false" do
(1..13).each{ |idx| tree.age!}
Copy link
Member

Choose a reason for hiding this comment

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

This works. You could also use .times which would make this cleaner.
https://ruby-doc.org/core-2.2.2/Integer.html#method-i-times

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants