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

tree.rb runs, added a few minor tests. Need to work on testing #57

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
GEM
remote: https://rubygems.org/
specs:
diff-lcs (1.2.5)
rspec (3.1.0)
Expand All @@ -19,3 +20,6 @@ PLATFORMS

DEPENDENCIES
rspec

BUNDLED WITH
1.16.0
29 changes: 23 additions & 6 deletions lib/tree.rb
Original file line number Diff line number Diff line change
@@ -1,25 +1,39 @@
class NoApplesError < StandardError; end

class AppleTree
attr_#fill_in :height, :age, :apples, :alive
class Tree
attr_accessor :height, :age, :apples, :alive

def initialize
@height = 0
@age = 0
@apples = []
@alive = true
end

def age!
@age += 1
@height += 1
5.times do
add_apples
end
end

def add_apples
apple = Apple.new("Red", rand(1..10))
apples.push(apple)
end

def any_apples?
@apples != []
Copy link
Member

Choose a reason for hiding this comment

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

@apples.empty? would work too, and it a bit more readable IMO.

end

def pick_an_apple!
raise NoApplesError, "This tree has no apples" unless self.any_apples?
@apples.pop
end

def dead?
self.age > 20
end
end

Expand All @@ -29,13 +43,16 @@ def initialize
end
end

class Apple <
attr_reader #what should go here
class Apple < Fruit
attr_reader :diameter, :color

def initialize(color, diameter)
@color = color
@diameter = diameter
end
end


#THERES ONLY ONE THING YOU NEED TO EDIT BELOW THIS LINE
# avg_diameter (line 58) will raise an error.
# it should calculate the diameter of the apples in the basket
Expand All @@ -61,7 +78,7 @@ def tree_data
diameter_sum += apple.diameter
end

avg_diameter = # It's up to you to calculate the average diameter for this harvest.
avg_diameter = diameter_sum / basket.length # It's up to you to calculate the average diameter for this harvest.

puts "Year #{tree.age} Report"
puts "Tree height: #{tree.height} feet"
Expand All @@ -76,4 +93,4 @@ def tree_data
end

# Uncomment this line to run the script, but BE SURE to comment it before you try to run your tests!
# tree_data
tree_data
42 changes: 38 additions & 4 deletions spec/tree_spec.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,48 @@
require 'rspec'
require 'tree'

describe 'Tree' do
describe Tree do
it 'should be a Class' do
expect(described_class.is_a? 'Class').to be_true
expect(described_class.is_a? Class).to eq true
end

it 'should have height' do
tree = Tree.new
Copy link

Choose a reason for hiding this comment

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

The idiomatic rspec way would be to use a let statement for tree here. Generally speaking, let statements are preferred over local variables.

Copy link
Member

@mikegee mikegee Dec 13, 2017

Choose a reason for hiding this comment

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

I kinda like local variables. ¯\_(ツ)_/¯

Extracting the repetition of tree = Tree.new is nice, but the clarity of the local variable has some benefit too.

expect(tree.height).to_not eq nil
end

it 'should age' do
tree = Tree.new
tree.age!
expect(tree.height).to eq 1
expect(tree.age).to eq 1
expect(tree.apples).to_not eq []
end

it 'should add apples' do
tree = Tree.new
tree.add_apples
expect(tree.apples).to_not eq []
end
Copy link

Choose a reason for hiding this comment

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

For a lot of these tests, you might do some thinking about what the subject of the test really is. Here, for instance, the subject is really the add_apples method, rather than the Tree class itself. So using a nested describe block would be a good way to represent that.

Also, think about whether this is the right test. Sometimes you might want to call add_apples on a tree that already had apples, in which case the fact that tree.apples is not empty is pretty meaningless.


it 'should have apples' do
tree = Tree.new
tree.add_apples
expect(tree.any_apples?).to eq true
end
end

describe 'Fruit' do
describe Fruit do
end

describe 'Apple' do
describe Apple do
it 'should be a Class' do
expect(described_class.is_a? Class).to eq true
end

it 'should create a new Apple' do
apple = Apple.new("Red", 5)
expect(apple.color).to eq "Red"
expect(apple.diameter).to eq 5
end
end