-
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
airport_challenge #2511
base: main
Are you sure you want to change the base?
airport_challenge #2511
Changes from all commits
c64b49e
9cbd866
d1f8976
b8ea842
a5a418b
aa08f70
66408f1
8efe7ae
71857c4
5585cdf
d6d1288
73fa3b6
5eb35b2
917ae7b
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,23 @@ | ||
class Airport | ||
attr_reader :plane | ||
|
||
def initialize(capacity) | ||
@capacity = capacity | ||
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. How would you implement a default capacity, in case the user didn't specify one? Maybe take a look at including a default argument: https://www.rubyguides.com/2018/06/rubys-method-arguments/ |
||
@planes = [] | ||
end | ||
|
||
def land(plane) | ||
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, simple names. I feel like I overcomplicated mine with land_plane and release_plane. |
||
raise 'Airport is full, cannot land plane' if capacity_full? | ||
@planes << (plane) | ||
end | ||
|
||
def takeoff(plane) | ||
raise 'cannot take off, weather is stormy' if 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. How can having a plane take off be reflected? You're currently storing landed planes in @planes - they need to be decremented from this array once they take off, to show they're no longer in the airport. 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. In order for Weather's stormy? method to be called here, you could refactor your class with the following snippets (as an example):
With the above, you would initialize a Weather instance with Weather.new, and pass that in as an argument when initialising Airport.new. Then in your land/take_off methods, you could write:
The last remaining step will be to refactor your Weather class itself - currently, the result will always be the same. Can you think of a way you might randomise the result, so that it's sometimes sunny and sometimes stormy? |
||
end | ||
|
||
def capacity_full? | ||
max_capacity = 10 | ||
@planes.size >= max_capacity | ||
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. Can't say without running it, but should this be @planes.size > max_capacity? I think this would make max_capacity 9 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. Additionally, you should use the capacity you passed in in the initialize method, stored in @capacity. Otherwise, it's a duplication of code, and max_capacity will remain static at 10 regardless of the user input. |
||
end | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class Plane | ||
def at_airport? | ||
false | ||
end | ||
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. Unused methods should be removed before opening a PR. |
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class Weather | ||
def stormy? | ||
true | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
require './docs/airport_challenge' | ||
require './docs/weather' | ||
require './docs/plane' | ||
|
||
describe Airport do | ||
it 'creates instance of airport' do | ||
airport = Airport.new(10) | ||
expect(airport).to be_kind_of(Airport) | ||
end | ||
|
||
# it 'allows plane to land' do | ||
# airport = Airport.new | ||
# expect(airport).to respond_to :land | ||
# end | ||
# not needed as it is fully covered by a future test | ||
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 can remove unneeded tests / commented out code / tests too. |
||
|
||
it 'land takes 1 argument' do | ||
airport = Airport.new(10) | ||
expect(airport).to respond_to(:land).with(1).argument | ||
end | ||
|
||
it 'allows a plane to takeoff from airport' do | ||
airport = Airport.new(10) | ||
expect(airport).to respond_to(:takeoff).with(1).argument | ||
end | ||
|
||
it 'raises error when landing at full airport' do | ||
airport = Airport.new(10) | ||
plane = Plane.new | ||
10.times do | ||
airport.land(plane) | ||
end | ||
expect { airport.land(plane) }.to raise_error 'Airport is full, cannot land plane' | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
require './docs/airport_challenge' | ||
require './docs/weather' | ||
require './docs/plane' | ||
|
||
describe Plane do | ||
it 'creates instance of plane' do | ||
plane = Plane.new | ||
expect(plane).to be_kind_of(Plane) | ||
end | ||
|
||
it 'show that plane is no longer at airport' do | ||
plane = Plane.new | ||
plane.at_airport? | ||
expect(plane.at_airport?).to be false | ||
end | ||
|
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
require './docs/airport_challenge' | ||
require './docs/weather' | ||
require './docs/plane' | ||
|
||
describe Weather do | ||
it 'creates instance of weather' do | ||
weather = Weather.new | ||
expect(weather).to be_kind_of(Weather) | ||
end | ||
|
||
it 'weather responds to stormy?' do | ||
weather = Weather.new | ||
expect(weather).to respond_to(:stormy?) | ||
end | ||
|
||
it 'is unable to takeoff if weather is stormy' do | ||
weather = Weather.new | ||
airport = Airport.new(10) | ||
weather.stormy? | ||
expect(airport.takeoff(@plane)).to raise_error 'cannot take off, weather is stormy' | ||
#how to raise error if stormy? is true??? | ||
end | ||
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.
Do you need to expose the plane variable outside of this class?