Skip to content

Time - Nataliya#18

Open
npogodina wants to merge 35 commits intoAda-C13:masterfrom
npogodina:master
Open

Time - Nataliya#18
npogodina wants to merge 35 commits intoAda-C13:masterfrom
npogodina:master

Conversation

@npogodina
Copy link

Assignment Submission: Hotel

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

Reflection

Question Answer
What was a design challenge that you encountered on this project? The most challenging part was adding hotel blocks (Wave 3) while keeping my classes loosely coupled. My initial thought was to create a Block object which will store an array of Room objects in a variable. At the same time Rooms also needed to know which Blocks they are part of, and this created some sort of circular dependancy. I ended up using Reservations for "set aside in a block" scenario which allowed me to avoid tight coupling.
What was a design decision you made that changed over time over the project? I initially kept all methods that had to do with reservations in Reservation Desk. I thought it's reasonable since Reservation Desk responsibility was defined as "to reserve rooms". Later, I realized that a lot of it had to do with individual rooms, so I moved pieces of those methods to Room class. It helped tidy up the ReservationDesk a bit and at the same time gave Room some behavior (before Room had no methods).
What was a concept you gained clarity on, or a learning that you'd like to share? I did Test First Development for the first time and loved it! I also learned how to use private methods and keyword arguments. I tried hard to implement OOD best practices (single responsibility, limit dependancies, the law of demeter), but I am unsure how good of a job I did. Definitely need more (much more) practice with those.
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case? ReservationDesk#reserve_from_block method was tested to make sure it returns true and adds the reservation to Room#reservations array. That what should happen in case of a successful reservation.
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case? ReservationDesk#reserve_from_block method was tested to make sure it raises Exceptions if either block ID or room ID are invalid, if room is not part of the block or if the room has already been reserved. These are less common cases. In reality users should not be passing those arguments, but mistakes can happen, and our goal is to bring them to our user's attention.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I charted a simplified diagram first, then moved on to tests and then to code. I only wrote pseudocode first for one or two methods which had nested loops and slightly complex conditions. I wish I would have spent a little longer on polishing up the diagram. I didn't think it through well enough and as a result had to re-arrange methods quite a few times. I am very happy about practicing Test First Development though!

npogodina added 30 commits March 2, 2020 12:40
@beccaelenzil
Copy link

Hotel

Section 1: Major Learning Goals

Criteria yes/no, and optionally any details/lines of code to reference
Practices SRP by having at least two separate classes with distinct responsibilities, and test files for these two classes ✔️Great work on this. It is clear by your reflection that you iterated on your design and made decision based on OOP design best practices. Nice work!
Overall, demonstrates understanding instance variables vs. local variables. (There aren't unnecessarily too many instance variables, when it should be a local variable) ✔️
For each test file, tests demonstrate an understanding of instantiating objects properly, and using Arrange-Act-Assert ✔️
Practices pseudocode and TDD, and reflected on it by filling out the reflection questions ✔️
Practices git with at least 15 small commits and meaningful commit messages ✔️

Section 2: Code Review and Testing Requirements

Criteria yes/no, and optionally any details/lines of code to reference
There is a class that represents a reservation, and a second class that holds/manages a collection of reservations through composition (instance variable) ✔️
The logic for checking if a reservation's date overlaps with another reservation's date is complex logic that is separated into method(s) (and potentially class(es)) ✔️
The logic for checking if a reservation's date overlaps with another reservation's date has unit tests ✔️
All of the given tests run and pass ✔️
A test coverage tool is installed and used, and shows 95% test coverage ✔️

Section 3: Feature Requirements

Feature Requirement: There is a method that... yes/no
gives back a list of rooms, and it's tested ✔️
creates a specific reservation for a room for a given date range, and it has nominal test cases ✔️
creates a specific reservation for a room for a given date range, and it tests an edge case, such as no available room, or invalid date range ✔️
gives back a list of reservations on a given date. Its tests include a test that makes several reservations for a given date ✔️
calculates the total price for a reservation ✔️
gives back a list of available rooms for a given date range, and it has nominal test cases ✔️
gives back a list of available rooms for a given date range, and it has edge test cases, such as no available room, or invalid date range ✔️
creates a block of rooms ✔️
reserves a room from a block ✔️

Overall Feedback

Overall Feedback Criteria yes/no
Green (Meets/Exceeds Standards) 14+ total in all sections

Additional Feedback

Great work overall! You've built your first project with minimal starting code. This represents an incredible milestone in your journey, and you should be proud of yourself!

I am particularly impressed by the way that you wrote multiple methods in the Room class to deal with the difficult task of creating a block. You clearly spent time considering different design options, and worked through some tricky logic. In addition, the thoroughness of your unit tests are quite impressive.

I've left a few inline comments for you to review, mainly focused on ways you might consider refactoring your tests. These notes are minor. This is an impressive project. Keep up the hard work!

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality Yes?
Perfect Indentation
Elegant/Clever
Descriptive/Readable
Logical/Organized


@date_range = DateRange.new(start_date: start_date, end_date: end_date)
@room_id = room_id
@rate = (rate == :default) ? 200 : rate

Choose a reason for hiding this comment

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

Nice use of a ternary, and an optional keyword argument

end

it "returns true if the dates overlap and false if not" do
# TODO: each one should be a new test?

Choose a reason for hiding this comment

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

Since each other this expects are testing a different way that dates could overlap, it would be good to put each of these assertions in their own it block that describes exactly what you are testing.

end

describe "rooms" do
it "returns an array" do

Choose a reason for hiding this comment

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

these two it blocks (returns and array and returns an array of Rooms) could be included in a single it block

end
end

describe "find_reservations" do

Choose a reason for hiding this comment

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

These tests are quite thorough. Nice work! Make sure to also include a test for an edge case such as no reservations for a given date range.

expect(@reservation_desk.rooms[2].available?(start_date: "2020-3-1", end_date: "2020-3-5")).must_equal false
end

it "No ID provided: raises an Exception if no rooms are available for requested dates" do

Choose a reason for hiding this comment

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

Great test of an edge case. Minor note: Consider making a custom exception for this situation.

end

describe "cost" do
it "returns an integer" do

Choose a reason for hiding this comment

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

These two it blocks could be combined into one.

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