Skip to content

Airport challenge - #2496

Open
NBenzineb wants to merge 2 commits into
makersacademy:mainfrom
NBenzineb:main
Open

Airport challenge#2496
NBenzineb wants to merge 2 commits into
makersacademy:mainfrom
NBenzineb:main

Conversation

@NBenzineb

@NBenzineb NBenzineb commented Mar 28, 2022

Copy link
Copy Markdown

Your name

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

  • User story 1: "I want to instruct a plane to land at an airport"
  • User story 2: "I want to instruct a plane to take off from an airport and confirm that it is no longer in the airport"
  • User story 3: "I want to prevent landing when the airport is full"
  • User story 4: "I would like a default airport capacity that can be overridden as appropriate"
  • User story 5: "I want to prevent takeoff when weather is stormy"
  • User story 6: "I want to prevent landing when weather is stormy"

README checklist

Does your README contains instructions for

  • how to install,
  • how to run,
  • and how to test your code?

Here is a pill that can help you write a great README!

Comment thread lib/airport.rb
def initialize(capacity=DEFAULT_CAPACITY)
@airplanes = []
@capacity = capacity
condition = Weather.new

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would maybe think about creating the Weather instance when the plane takes off/lands, rather than creating the weather the same time the airport is created. As the weather will most likely not be the same as it was the day the airport was created.

Comment thread lib/airport.rb
end

def land_plane(airplane)
fail "Airport is full" if airplanes.length == capacity

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good :)

Comment thread lib/airport.rb

def land_plane(airplane)
fail "Airport is full" if airplanes.length == capacity
fail "Can't land as weather is stormy" unless sunny

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I can't see the instance variable sunny relating to anything, should it be weather.sunny?

Comment thread lib/airport.rb
fail "Can't land as weather is stormy" unless sunny
fail "Airplane is already here" if airplane.landed
airplane.landed = true
airplanes << airplane

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This seems good!

Comment thread lib/airport.rb
end

def takeoff_plane(airplane)
fail "Weather Stormy cannot take off" unless sunny

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same comment as above regarding sunny instance variable

Comment thread spec/airport_spec.rb
subject.sunny = true
allow(airplane).to receive(:landed).and_return(false)
Airport::DEFAULT_CAPACITY.times { subject.land_plane(airplane) }
expect{subject.land_plane(airplane)}.to raise_error "Airport is full"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

good :)

Comment thread spec/airport_spec.rb
expect{subject.land_plane(airplane)}.to raise_error "Airport is full"
end

it "Check to see if you can fill, remove then fill airport" do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This test description seems a little confusing

Comment thread spec/airport_spec.rb
end

it "Overwrite default airport capacity to 30" do
expect(subject.capacity=30).to eq 30

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could also test a new instance of airport with capacity as paramater. I would also consider changing this number, as the default capacity is already set to 30

Comment thread spec/plane_spec.rb
require '../lib/plane.rb'

describe Plane do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Here you could add some tests to see if the plane is in the air or if it's landed

Comment thread spec/weather_spec.rb
let(:weather) {double :weather, :sunny= => true, sunny?: true}

it "Check weather = sunny" do
expect(weather).to be_sunny

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants