Skip to content
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 #2496

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 54 additions & 35 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Airport Challenge
```
______
_\____\___
= = ==(____MA____)
= = ==(____NB____)
\_____\___________________,-~~~~~~~`-.._
/ o o o o o o o o o o o o o o o o |\_
`~-.__ __..----..__ )
Expand All @@ -13,26 +13,41 @@ Airport Challenge

```

Instructions
---------
# Airport Challenge

* Feel free to use google, your notes, books, etc. but work on your own
* If you refer to the solution of another coach or student, please put a link to that in your README
* If you have a partial solution, **still check in a partial solution**
* You must submit a pull request to this repo with your code by 10am Monday morning
This Airport challenge allows users to instruct a plane to land and take off from an airport, but only if the weather is sunny! We have a request from a client to write the software to control the flow of planes at an airport. The planes can land and take off provided that the weather is sunny. Occasionally it may be stormy, in which case no planes can land or take off.

Steps
-------
## The task

1. Fork this repo, and clone to your local machine
2. Run the command `gem install bundler` (if you don't have bundler already)
3. When the installation completes, run `bundle`
4. Complete the following task:
My task was to test drive the creation of a set of classes/modules to satisfy all the above user stories. I needed to use a random number generator to set the weather (it is normally sunny but on rare occasions it may be stormy). In my tests, I needed need to use a stub to override random weather to ensure consistent test behaviour.

Task
-----
My code needed to defend against edge cases such as inconsistent states of the system ensuring that planes can only take off from airports they are in; planes that are already flying cannot take off and/or be in an airport; planes that are landed cannot land again and must be in an airport, etc.

We have a request from a client to write the software to control the flow of planes at an airport. The planes can land and take off provided that the weather is sunny. Occasionally it may be stormy, in which case no planes can land or take off. Here are the user stories that we worked out in collaboration with the client:
## Getting started

`git clone https://github.com/NBenzineb/airport_challenge.git`

## Usage

`irb -r ./lib/plane.rb'`
- You can create plane instances (plane = Plane.new) and airport instances (airport = Airport.new).
- To instruct a plane to take off from an airport you can use the 'takeoff' method (airport.takeoff(plane))
- To instruct a plane to land you can use the 'land' method (airport.land(plane))
### Some things to note
- The airport's default capacity is set to 10 planes, so if you would like to change the airports default capacity, you can pass your desired capacity as an integer to the airport instance eg (airport = Airport.new(20))
- You will only be able to land and take-off planes if the weather is sunny. Luckily, the weather is sunny 75% of the time, and stormy the other 25%.



## Running tests

`rspec`



## My approach to solving the problem

Here are the user stories that worked out in collaboration with the imaginary client.

```
As an air traffic controller
Expand Down Expand Up @@ -60,30 +75,34 @@ To ensure safety
I want to prevent landing when weather is stormy
```

Your task is to test drive the creation of a set of classes/modules to satisfy all the above user stories. You will need to use a random number generator to set the weather (it is normally sunny but on rare occasions it may be stormy). In your tests, you'll need to use a stub to override random weather to ensure consistent test behaviour.

Your code should defend against [edge cases](http://programmers.stackexchange.com/questions/125587/what-are-the-difference-between-an-edge-case-a-corner-case-a-base-case-and-a-b) such as inconsistent states of the system ensuring that planes can only take off from airports they are in; planes that are already flying cannot take off and/or be in an airport; planes that are landed cannot land again and must be in an airport, etc.

For overriding random weather behaviour, please read the documentation to learn how to use test doubles: https://www.relishapp.com/rspec/rspec-mocks/docs . There’s an example of using a test double to test a die that’s relevant to testing random weather in the test.

Please create separate files for every class, module and test suite.

In code review we'll be hoping to see:
### Step 1: Functional representation of user stories

* All tests passing
* High [Test coverage](https://github.com/makersacademy/course/blob/main/pills/test_coverage.md) (>95% is good)
* The code is elegant: every class has a clear responsibility, methods are short etc.
To get me started, I identified all the nouns and verbs in the user stories.

Reviewers will potentially be using this [code review rubric](docs/review.md). Referring to this rubric in advance will make the challenge somewhat easier. You should be the judge of how much challenge you want this at this moment.
I then considered for each user story which of those nouns and verbs relate to the end users of the software and what they need to be able to do with it, and which were about how the software itself needs to function, i.e. pertaining to the **functional representation** of the user stories in terms of **objects** in the program.

**BONUS**
I mapped out functional representation of each story in terms of _objects_, _messages_, _parameters_, _behaviour_, and where more relevant, _objects_, _constants_, and _instance variables_.

* Write an RSpec **feature** test that lands and takes off a number of planes
Here's a consolidated mapping for an overall picture of the structure of of the program as a whole, as follows:

Note that is a practice 'tech test' of the kinds that employers use to screen developer applicants. More detailed submission requirements/guidelines are in [CONTRIBUTING.md](CONTRIBUTING.md)
**Objects and messages**

Finally, don’t overcomplicate things. This task isn’t as hard as it may seem at first.
| Objects | Messages | Parameters | Behaviour |
| ------------| ----------------| -------------| -----------------------------|
| Weather | Stormy? | | Reports whether stormy |
| Airport | Full? | | Reports whether full |
| Airport | Change capacity | | Changes capacity |
| Plane | Land | Airport | Lands at airport |
| | | | Prevents landing if stormy |
| | Take off | Airport | Take off from airport |
| | | | Prevents take-off if stormy |
| | Report | | Report location |

* **Submit a pull request early.**
**Objects and states**

* Finally, please submit a pull request before Monday at 10am with your solution or partial solution. However much or little amount of code you wrote please please please submit a pull request before Monday at 10am.
| Objects | Constants | Instance variables |
| ------------| -------------------| ---------------------|
| Weather | Weather types | Stormy |
| Airport | Default capacity | Capacity |
| | | Hangar |
| Plane | | Location |
Empty file added lib/airport
Empty file.
43 changes: 43 additions & 0 deletions lib/airport.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
require_relative "../lib/plane.rb"
require_relative "../lib/weather.rb"

class Airport

attr_reader :airplanes
attr_accessor :capacity, :sunny
DEFAULT_CAPACITY = 30

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.

@weather = condition.sunny
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 :)

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 "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

fail "Airplane is already in the sky!" unless airplane.landed
return airplane if check_plane(airplane) == airplane
fail "Airplane is not at this airport"
end

private

def check_plane(airplane)
airplanes.each_with_index do |check,index|
next unless check == airplane
airplanes.delete_at(index)
airplane.landed = false
return airplane
end
end

end
11 changes: 11 additions & 0 deletions lib/plane.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Plane
attr_accessor :landed

def initialize(status=true)
@landed = status
end

def landed?
landed

Choose a reason for hiding this comment

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

This seems good, you could probably add a method for take-off also, as the plane has two objectives, to land and take off.

end
end
11 changes: 11 additions & 0 deletions lib/weather.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class Weather
attr_reader :sunny

def initialize
rand(10) == 0 ? @sunny = false : @sunny = true
end

def sunny?
sunny

Choose a reason for hiding this comment

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

Maybe add a stormy method also, if it's not sunny, what is the weather?

end
end
93 changes: 93 additions & 0 deletions spec/airport_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@

require '../lib/airport.rb'
require '../lib/plane.rb'

describe Airport do
let(:airplane) {double :airplane, :landed= => false, landed?: false}
let(:weather) {double :weather, :sunny= => true, sunny?: true}

Choose a reason for hiding this comment

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

Good job using doubles :)


it "Land a plane at the airport and confirm" do
subject.sunny = true
allow(airplane).to receive(:landed).and_return(false)
expect(subject.land_plane(airplane)).to include(airplane)
end

it "Land plane then take off" do

Choose a reason for hiding this comment

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

Maybe change the description to something like 'takes off plane after it has landed' or something similar. This description seems like you're testing the landing and the take off in the same test

subject.sunny = true
allow(airplane).to receive(:landed).and_return(false)
subject.land_plane(airplane)
allow(airplane).to receive(:landed).and_return(true)
expect(subject.takeoff_plane(airplane)).to eq airplane
end

it "Check plane is in sky and not landed" do
subject.sunny = true
allow(airplane).to receive(:landed).and_return(false)

Choose a reason for hiding this comment

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

You could maybe add a before each block at the start of your spec for this, as it seems to be used quite often, if you like

subject.land_plane(airplane)
allow(airplane).to receive(:landed).and_return(true)
subject.takeoff_plane(airplane)
expect(airplane).to_not be_landed

Choose a reason for hiding this comment

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

good :)

end

it "Make sure plane that as taken off is not at the airport" do

Choose a reason for hiding this comment

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

i think there is a typo in here, has instead of as

subject.sunny = true
allow(airplane).to receive(:landed).and_return(false)
subject.land_plane(airplane)
allow(airplane).to receive(:landed).and_return(true)
subject.takeoff_plane(airplane)
expect(subject.airplanes).to_not include(airplane)

Choose a reason for hiding this comment

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

good test

end

it "Prevent plane to take off if not sunny" do
subject.sunny = true
allow(airplane).to receive(:landed).and_return(false)
subject.land_plane(airplane)
subject.sunny = false
error = "Weather Stormy cannot take off"
expect{subject.takeoff_plane(airplane)}.to raise_error error

Choose a reason for hiding this comment

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

You could also add the error message in this line. 'raise_error "Weather stormy cannot take off"' - rather than having this over two lines

end

it "Prevent airplane to land if not sunny" do
subject.sunny = false
error = "Can't land as weather is stormy"
expect{subject.land_plane(airplane)}.to raise_error error

Choose a reason for hiding this comment

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

same comment as above

end

it "Raise an error if the airport is full" do
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 :)

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

subject.sunny = true
allow(airplane).to receive(:landed).and_return(false)
Airport::DEFAULT_CAPACITY.times { subject.land_plane(airplane) }
allow(airplane).to receive(:landed).and_return(true)
subject.takeoff_plane(airplane)
expect(subject.airplanes).to include(airplane)
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

end

it "Raise error if plane already in sky and try takeoff" do
subject.sunny = true
allow(airplane).to receive(:landed).and_return(false)
error = "Airplane is already in the sky!"
expect{subject.takeoff_plane(airplane)}.to raise_error error
end

it "Raise error if plane already tries to land when already at airport" do
subject.sunny = true
allow(airplane).to receive(:landed).and_return(false)
subject.land_plane(airplane)
allow(airplane).to receive(:landed).and_return(true)
error = "Airplane is already here"
expect{subject.land_plane(airplane)}.to raise_error error
end

end
5 changes: 5 additions & 0 deletions spec/plane_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
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

end
16 changes: 16 additions & 0 deletions spec/weather_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
require "../lib/weather.rb"

describe Weather do

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

end

let(:weather2) {double :weather, :sunny= => false, sunny?: false}
it "Check weather != sunny" do
allow(weather2).to receive(:sunny).and_return(false)
expect(weather2).to_not be_sunny
end
end