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

Tom O'Neill airport challenge #2519

Open
wants to merge 5 commits into
base: main
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
46 changes: 46 additions & 0 deletions lib/airport.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
require_relative 'plane'

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


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'

Choose a reason for hiding this comment

The 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'

Choose a reason for hiding this comment

The 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 == []

Choose a reason for hiding this comment

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

You could use .empty? which might be cleaner

end

end
2 changes: 2 additions & 0 deletions lib/plane.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class Plane
end
55 changes: 55 additions & 0 deletions spec/airport_spec.rb
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) }

Choose a reason for hiding this comment

The 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

Choose a reason for hiding this comment

The 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

let(:plane) { Plane.new }

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

Choose a reason for hiding this comment

The 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
5 changes: 5 additions & 0 deletions spec/plane_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
require 'plane'

describe Plane do

end