-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Tom O'Neill airport challenge #2519
base: main
Are you sure you want to change the base?
Changes from all commits
306de48
9def946
25b4e97
778872d
0cd7347
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
require_relative 'plane' | ||
|
||
class Airport | ||
attr_reader :stored_planes, :weather | ||
CAPACITY = 5 | ||
|
||
def initialize | ||
@stored_planes = [] | ||
@capacity = CAPACITY | ||
@weather = self.set_weather | ||
end | ||
|
||
def land(plane) | ||
fail 'Airport full!' if full? | ||
fail 'Cannot land - stormy weather!' if @weather == 'Stormy' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider refactoring the condition into it's own method (SRP) |
||
@stored_planes << plane | ||
end | ||
|
||
def depart(plane) | ||
fail 'No planes' if empty? | ||
fail 'Plane not in airport' if !@stored_planes.include?(plane) | ||
@stored_planes.delete(plane) | ||
end | ||
|
||
def adjust_capacity(number) | ||
@capacity = number | ||
end | ||
|
||
def set_weather | ||
die = rand(100) | ||
die > 100 ? 'Stormy' : 'Sunny' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider if there is a way to write this without the ternary |
||
end | ||
|
||
private | ||
|
||
attr_accessor :capacity | ||
|
||
def full? | ||
@stored_planes.length >= @capacity | ||
end | ||
|
||
def empty? | ||
@stored_planes == [] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could use .empty? which might be cleaner |
||
end | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
class Plane | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
require 'airport' | ||
|
||
describe Airport do | ||
|
||
describe '#land' do | ||
it 'Can do so' do | ||
plane = Plane.new | ||
expect(subject.land(plane)).to eq([plane]) | ||
end | ||
|
||
it 'Will not do so if the airport is full' do | ||
Airport::CAPACITY.times { subject.land(Plane.new) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good use of constant instead of magic number |
||
expect { subject.land(Plane.new) }.to raise_error 'Airport full!' | ||
end | ||
|
||
end | ||
|
||
describe '#depart' do | ||
it 'Can instruct a stored plane to take off from the airport' do | ||
plane = Plane.new | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you are initialising a plane multiples times in the spec, you could consider using let to define a plane variable and then refer to that
|
||
subject.land(plane) | ||
subject.depart(plane) | ||
expect(subject.stored_planes).to eq [] | ||
end | ||
|
||
it 'Will not do so if there are no planes in the airport' do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could reword the it statement to be less wordy and easier to read, perhaps by using should ( e.g. 'should not depart if there are no planes') or consider a context block to set the stage for multiple tests where the airport is empty. |
||
expect { subject.depart(Plane.new) }.to raise_error 'No planes' | ||
end | ||
|
||
it 'Will not take off if the plane is not in the airport' do | ||
subject.land(Plane.new) | ||
expect{subject.depart(Plane.new)}.to raise_error 'Plane not in airport' | ||
end | ||
end | ||
|
||
it 'can have an airport capacity that can be overwritten as appropriate' do | ||
subject.adjust_capacity(1) | ||
subject.land(Plane.new) | ||
expect { subject.land(Plane.new) }.to raise_error 'Airport full!' | ||
end | ||
|
||
it 'has a default capacity' do | ||
Airport::CAPACITY.times { subject.land(Plane.new) } | ||
expect { subject.land(Plane.new) }.to raise_error 'Airport full!' | ||
end | ||
|
||
describe 'initialize' do | ||
it 'will be stormy weather and the plane will not land' do | ||
allow(subject).to receive(:set_weather).and_return('Stormy') | ||
expect { subject.land(Plane.new) }.to raise_error 'Cannot land - stormy weather!' | ||
end | ||
end | ||
|
||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
require 'plane' | ||
|
||
describe Plane do | ||
|
||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Believe if you require the files in the spec_helper you don't need to require them on each individual file as well