airport_challenge #2511
airport_challenge #2511
Conversation
|
|
||
| def capacity_full? | ||
| max_capacity = 10 | ||
| @planes.size >= max_capacity |
There was a problem hiding this comment.
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.
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.
| @planes = [] | ||
| end | ||
|
|
||
| def land(plane) |
There was a problem hiding this comment.
Good, simple names. I feel like I overcomplicated mine with land_plane and release_plane.
| @@ -0,0 +1,23 @@ | |||
| class Airport | |||
| attr_reader :plane | |||
There was a problem hiding this comment.
Do you need to expose the plane variable outside of this class?
| attr_reader :plane | ||
|
|
||
| def initialize(capacity) | ||
| @capacity = capacity |
There was a problem hiding this comment.
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/
| end | ||
|
|
||
| def takeoff(plane) | ||
| raise 'cannot take off, weather is stormy' if stormy? |
There was a problem hiding this comment.
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.
In order for Weather's stormy? method to be called here, you could refactor your class with the following snippets (as an example):
def initialize(capacity, weather)
@capacity = capacity
@weather = weather
def
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:
raise 'cannot take off, weather is stormy' if @weather.stormy?
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?
|
|
||
| def capacity_full? | ||
| max_capacity = 10 | ||
| @planes.size >= max_capacity |
There was a problem hiding this comment.
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.
| class Plane | ||
| def at_airport? | ||
| false | ||
| end |
There was a problem hiding this comment.
Unused methods should be removed before opening a PR.
| # 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.
You can remove unneeded tests / commented out code / tests too.
Sam Button
User stories
Please list which user stories you've implemented (delete the ones that don't apply).
README checklist
Does your README contains instructions for
Here is a pill that can help you write a great README!