Skip to content

Conversation

@NBenzineb
Copy link

@NBenzineb NBenzineb commented Mar 28, 2022

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!

def initialize(capacity=DEFAULT_CAPACITY)
@airplanes = []
@capacity = capacity
condition = Weather.new

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.

end

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

Choose a reason for hiding this comment

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

Good :)


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

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?

fail "Can't land as weather is stormy" unless sunny
fail "Airplane is already here" if airplane.landed
airplane.landed = true
airplanes << airplane

Choose a reason for hiding this comment

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

This seems good!

end

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

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

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"

Choose a reason for hiding this comment

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

good :)

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

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

end

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

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

require '../lib/plane.rb'

describe Plane do

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

let(:weather) {double :weather, :sunny= => true, sunny?: true}

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

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