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

David Campbell - Airport_Challenge - 3rd User Story - 4th Commit #2498

Open
wants to merge 6 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
2 changes: 1 addition & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Your name
# Your name: David Ewan Campbell

Please write your full name here to make it easier to find your pull request.

Expand Down
45 changes: 45 additions & 0 deletions README_David_Campbell_Airport_Challenge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
David Campbell - March '22 cohort
Makers Course WEEK 2 - AIRPORT CHALLENGE

- Create a small program to control flow of planes at an airport.
- Every plane has a status to say wether it is in the air or landed at the airport
- This program developed using TDD

# initialised local git repo
# original airport challenge repo forked in github then cloned from github to local repo

# ruby updated to 3.0.2
# gem bundler installed/updated
# checked rspec up-to-date

User story 1 - 3 covered
# As an air traffic controller
# So I can get passengers to a destination
# I want to instruct a plane to land at an airport

# So I can get passengers on the way to their destination
# I want to instruct a plane to take off from an airport and confirm that it is no longer in the airport

# To ensure safety
# I want to prevent landing when the airport is full

-Objectives, turn user stories into objects & actions

Choose a reason for hiding this comment

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

I made a basic description of adapting the user story process up to the point I'd reached but maybe should have described the whole task and broken it down more clearly to help with the TDD process? Also to show how you take a client request and turn it from a user story into specific code that handles that request?


object => airport(therefore destination) => plane(object)in air, action: tell plane to land, take off, cannot land when airport full...

object => plane => action: landed at airport

- rspec spec/airport_spec.rb to run tests from airport_challenge folder
calling Class defined in airport.rb file

README revised on third commit to make process taken clearer and concentrated on working with airport spec and airport object/class (deleted created plane class & spec file at this stage as over-complicating process)

Choose a reason for hiding this comment

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

Should have created plane object? Put A Plane Class in a separate file and require that file from other files...


# With each step I ran a feature test first in irb then an rspec unit test, making sure they failed first and read result of each failed test. I created wrapper examples in my airport_spec then created each definition in my Airport class accordingly, making sure each step was failing then passing and ensuring 100 % coverage

# then ran rubocop and tightened up code accordingly - no offences detected and added and commited after each method definition added passed unit tests

-4th commit before pull request - added airport_capacity to limit number of planes allowed to land in airport
- renamed and moved README file to root folder
- ran rubocop to refactor/tidy code layout(one offence not corrected yet - unused method argument - will correct next session)

- rspec unit tests 3 examples passing with 100% coverage
14 changes: 14 additions & 0 deletions lib/airport.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
class Airport
def initialize(airport_capacity)
@airport_capacity = airport_capacity
@number_of_planes = 0
end

def land_plane(plane)
raise 'No space at airport for plane to land' if @number_of_planes == @airport_capacity

Choose a reason for hiding this comment

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

Would a >= be better here to protect against edge cases?

Choose a reason for hiding this comment

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

Yes, that's really good advice! The sort of thing spec/rubocop not throwing up and need to look out for myself.

Choose a reason for hiding this comment

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

Thank you!

@number_of_planes += 1
end

def plane_take_off(plane)
end

Choose a reason for hiding this comment

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

Had not got to stage of using plane_take_off definition to start making space in the airport and le other planes land/take off and keep track of the state of airport_capacity in the counter?

end
4 changes: 4 additions & 0 deletions pull_request_teamplate.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
David Campbell Pull request for Makers Academy Airport_Challenge
4th commit with added airport_capacity
one rubocop offence to correct with unused method argument
100 % coverage all tests pass
19 changes: 19 additions & 0 deletions spec/airport_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
require 'airport'

describe Airport do
subject(:airport) { airport = Airport.new(10) }
it 'can instruct plane to land' do

Choose a reason for hiding this comment

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

I don't think this test is sufficient to test your production code.

Your production code adds one to @number_of_planes but your test only looks to see if airport responds to land_plane.

Choose a reason for hiding this comment

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

No you're absolutely right Alexis, I removed a plane object I'd created in a separate file but needed to be working with a plane object and have that respond...

Choose a reason for hiding this comment

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

It was passing the most basic tests at this stage and maybe I was focusing on that '100 %' coverage passing each time and was just trying to get to grips with the feature/unit processes up to this stage BUT definitely not a strong test as far as the rest of the challenge and needed a lot more work. Will be fascinating to work on the process together this afternoon as I really want to vastly improve my process but still feel like I'm just getting to some sort of understanding of the basics...

expect(airport).to respond_to(:land_plane)
end

it 'can instruct plane to take off' do
expect(airport).to respond_to(:plane_take_off)

Choose a reason for hiding this comment

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

Again, assuming you want to build plane_take_off to remove 1 from your number_of_planes, then your test needs to be stronger.

Choose a reason for hiding this comment

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

Yes definitely that was my next step but ran out of time...

Choose a reason for hiding this comment

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

I hop leaving comments unresolved means this relates to the issue you've raised with my code where I need to be building on the next step and making that test stronger for that next step... great advice Alexis

end

it 'does not allow planes to land when at capacity' do
10.times do
airport.land_plane(:plane)

Choose a reason for hiding this comment

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

Created a plane 'test-double' to test code without having created a plane object? What are the benefits/advantages at the test stage with creating a 'double' at the early test stage compared with creating another plane object in this case?

Choose a reason for hiding this comment

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

What I did - and it may not be best practice - is to use actual objects of other classes in the tests initially. Then I was going to replace them later with doubles (but I ran out of time to do that).

At least initially, when the code is simple, using actual instances of other classes helped me to build the code across multiple classes that worked.

Maybe when you are immediately diving into a complex program, or improving on an existing complex program it's better to go straight for doubles.

Choose a reason for hiding this comment

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

Yes, I can see the advantages of doubles but as the remit definitely should have involved the creation of a plane object (which I did initially) though as you say that can come later if you are building a complex program.

Yes it makes sense to build instances of classes that can start working across your code...

end
expect { airport.land_plane(:plane).to raise_error 'No space at airport for plane to land' }
end
end