-
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
Gawain Hewitt's Airport Weekend Challenge, week 1 #2508
base: main
Are you sure you want to change the base?
Conversation
…to show that I got to the end of story 3 this way before I start again
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.
A good interpretation of Airport Challenge, well done.
You successfully met the specification of each user story, bar Weather, with some good examples of unit testing.
There was separation of concerns between Plane and Airport as objects.
With weather, it appears that the only circumstance in which the weather will be stormy is in your tests - can you think of a way of randomising the weather conditions, so that sometimes it will be stormy, and sometimes sunny?
@@ -0,0 +1,17 @@ | |||
class Airport | |||
attr_writer :capacity | |||
DEFAULT_CAPACITY = 20 |
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.
I'd leave an empty line between attr_readers, constant variables and the first method definition for readability
end | ||
|
||
def plane_take_off | ||
@hangar_log -= 1 |
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.
What would happen if this method was run when no planes had ever landed?
def land(airport) | ||
fail "plane already landed" if @landed | ||
fail "landing is prevented in stormy weather" if @weather == :stormy | ||
@landed = airport |
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.
'landed' as a variable description doesn't necessarily match up with airport as a value (in terms of its naming), when landed is a boolean value
it 'prevents landing when full' do | ||
20.times do | ||
plane = Plane.new | ||
planes << plane |
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.
You don't use the planes variable again after this - is it needed?
let(:ibiza) { Airport.new } | ||
let(:new_york) { Airport.new } | ||
|
||
let(:eoins_private_jet) { Plane.new } |
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.
Thank you very much 😎
Gawain Hewitt
Please write your full name here to make it easier to find your pull request.
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!