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

Initial add #54

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Initial add #54

wants to merge 1 commit into from

Conversation

andrewpetit
Copy link

No description provided.

@andrewpetit
Copy link
Author

@jaybobo

end

def add_apples
rn = Random.new
Copy link
Member

Choose a reason for hiding this comment

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

You don't have to initialize a new generator to use rand. It's an alias for Random::DEFAULT.rand so you can get away with just writing rand(1..10) here.

end

def any_apples?
if(@apples == nil || @apples.count <= 0)
return false
Copy link
Member

Choose a reason for hiding this comment

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

no need for explicit returns here.

end

def any_apples?
if(@apples == nil || @apples.count <= 0)
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 we want to use the getter here instead and anywhere else we can in this class?

end
end

class Tree < AppleTree
Copy link
Member

Choose a reason for hiding this comment

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

Slight nitpick. I might rename this with AppleTree inheriting from Tree.

end
end

describe 'Fruit' do
describe AppleTree do
Copy link
Member

Choose a reason for hiding this comment

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

Take a look at contexts. I would use those to refactor my tests into a story with a clear happy path and not so happy path.

www.betterspecs.org/#contexts

end

def dead?
if(@age > 50)
Copy link
Member

Choose a reason for hiding this comment

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

You could also write this...

  def dead?
    return true if age > 50
    false
  end


it 'dead? returns true if greater than 50' do
appleTree = AppleTree.new(1, 55, [], true)
expect(appleTree.dead?).to eq true
Copy link
Member

Choose a reason for hiding this comment

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

rspec also has dynamic predicate matchers which are pretty cool.

https://relishapp.com/rspec/rspec-expectations/v/3-6/docs/built-in-matchers/predicate-matchers

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.

2 participants