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

Conversation

david-ewan-campbell
Copy link

Your name

Please write your full name here to make it easier to find your pull request.
David Ewan Campbell (Ramble-ramble-70)

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!

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

# 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?


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

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?


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.

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.

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!


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

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

Copy link
Author

@david-ewan-campbell david-ewan-campbell left a comment

Choose a reason for hiding this comment

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

Intend on following up unresolved comments as they indicate very helpful advice on ways I could work on and improve the strength of my tests.

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