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

Russell's Airport Challenge #2504

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

class Airport
DEFAULT_CAPACITY = 1

attr_reader :capacity

def initialize(capacity = DEFAULT_CAPACITY)

Choose a reason for hiding this comment

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

I think you can just give whatever value you want default to be here, like capacity = 1, without needing to set the DEFAULT_CAPACITY variable. As with all of my comments, apologies if I'm entirely wrong....

@airport = []
@capacity = capacity
end

def land(plane)
fail 'Airport full' if full?
fail "Flight cannot land due to stormy weather" if bad_weather?
airport << plane
end

def take_off(*)
fail "Flight cancelled due to bad weather" if bad_weather?
@airport.pop

Choose a reason for hiding this comment

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

I guess .pop could be tricky if we had multiple planes in the airport, as it'll always just pop the most recent entry to the airport array. FWIW, I used .delete(plane)

end

def bad_weather?
weather.stormy?
end

def weather
@weather ||= Weather.new

Choose a reason for hiding this comment

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

Oo, nice - ||= is new to me

end

private

attr_reader :airport

def full?
airport.count >= capacity
end
end
3 changes: 3 additions & 0 deletions lib/plane.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Plane

end
13 changes: 13 additions & 0 deletions lib/weather.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

class Weather

Choose a reason for hiding this comment

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

Oh, nice - I didn't break weather into a separate class, but definitely should have. Good compartmentalisation


def stormy?
forecast >= 6
end

private

Choose a reason for hiding this comment

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

As an open question, how come you used a private method here? I don't really understand private methods, so would be great to know


def forecast
rand(10)
end
end
114 changes: 114 additions & 0 deletions spec/airport_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
require 'airport'
require 'weather'

describe Airport do

describe '#land' do

it 'instructs a plane to land at an airport' do

# User story 1

Choose a reason for hiding this comment

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

This is a nice way to make clear what you're testing. Didn't even occur to me

# As an air traffic controller
# So I can get passengers to a destination
# I want to instruct a plane to land at an airport

# Arrange
airport = Airport.new

Choose a reason for hiding this comment

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

FWIW, you can use subject instead of declaring airport = Airport.new each time, when the describe for the test gives the class name

plane = Plane.new
# Act / Assert
expect { airport.land(plane) }.not_to raise_error
end

it 'prevents a plane from landing when airport is full' do

# User story 3
# As an air traffic controller
# To ensure safety
# I want to prevent landing when the airport is full

subject.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.

Like the use of the capacity variable here. Pretty sure I hardcoded a value

expect { subject.land Plane.new }.to raise_error 'Airport full'
end

it 'does not allow planes to land with stormy weather' do

# User story 6
# As an air traffic controller
# To ensure safety
# I want to prevent landing when weather is stormy

# Arrange
airport = Airport.new
stormy_weather = Weather.new
plane = Plane.new

Choose a reason for hiding this comment

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

FWIW, you can do let(:plane) { Plane.new } underneath describe Airport do, then you don't have to do plane = Plane.new inside each test

Copy link
Author

Choose a reason for hiding this comment

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

Super helpful cheers.


# Act / Assert

allow(stormy_weather).to receive(:stormy?) { true }
allow(airport).to receive(:weather) { stormy_weather }

expect { airport.land(plane) }.to raise_error "Flight cannot land due to stormy weather"
end

end

describe '#take_off' do

it 'instructs a plane to take off and report it is not in the airport' do

# User Story 2
# As an air traffic controller
# So I can get passengers on the way to their destination
# I want to instruct a plane to take off from an airport and confirm that it is no longer in the airport

# Arrange
airport = Airport.new
plane = Plane.new

# Act / Assert
expect(subject.take_off).to eq 'Plane no longer in airport'
end

it 'prevents take off when weather is stormy' do

# User story 5
# As an air traffic controller
# To ensure safety
# I want to prevent takeoff when weather is stormy

# Arrange
airport = Airport.new
stormy_weather = Weather.new
plane = Plane.new

# Act / Assert

allow(stormy_weather).to receive(:stormy?) { true }
allow(airport).to receive(:weather) { stormy_weather }

expect { airport.take_off(plane) }.to raise_error "Flight cancelled due to bad weather"
end

end

it 'has a default airport capacity that can be overidden' do

# User story 4
# As the system designer
# So that the software can be used for many different airports
# I would like a default airport capacity that can be overridden as appropriate

expect(subject.capacity).to eq Airport::DEFAULT_CAPACITY
end

describe 'initialization' do
subject { Airport.new }
let(:plane) { Plane.new }
it 'defaults capacity' do
described_class::DEFAULT_CAPACITY.times do
subject.land(plane)
end
expect { subject.land(plane) }.to raise_error 'Airport full'
end
end
end