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

Kee Wei Airport Challenge #182

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

Saphly
Copy link

@Saphly Saphly commented Jun 18, 2023

No description provided.

Copy link
Author

@Saphly Saphly Jun 21, 2023

Choose a reason for hiding this comment

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

Like:

  • Group tests together by their situation, e.g. "When constructing an airport". This makes is feels more like a story and its easier for me to keep track.
  • Used switch case for checking the weather so that it would be easier to add another weather type if needed.
  • Classes are loosely coupled

Could have done better:

  • Have a better structured domain model. See how they all connect.
  • Have error handling instead of making the code do nothing or return nothing
  • to not test implementation details. e.g this.listOfPlanes.length.
  • After refactoring the code, go back and look at the test suite and see what could be changed.
    e.g. when landing a plane, should check that the object is actually a plane: could have used canLand() to check

Things to try to do to improve in next challenege:

  • Write out domain models for everything. Review it at the end to have a better feel on how they will interact/connect to each other.
  • Implement error handling
  • Enforce TDD
  • Make tests to be loosely coupled by using mock classes

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.

1 participant