Pipes - Sara Frandsen - Hotel

[frankienakama]

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
Describe a design decision you had to make when working on this project. What options were you considering? What helped you make your final decision?	I almost made two classes for the dates, one for check-in, and one for check-out. It was eventually decided that it was extraneous and just one date_range class was needed.
Describe a concept that you gained more clarity on as you worked on this assignment.	class methods and class variables, TDD, and raising exceptions
Describe a nominal test that you wrote for this assignment.	reservations_spec tested if an error was raised when a room was already booked.
Describe an edge case test that you wrote for this assignment.	reservation_spec will also make sure that a failed room assignment did not somehow get added to the array of all reservations
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	it helped A LOT, it felt like sketching a drawing. I referred to my original notes quite a bit. Though, I had to make so many changes, I did not end up finishing wave 3 :(

[droberts-sea]

Hotel

What We're Looking For

Feature	Feedback
Baseline
Used git regularly	yes - good work
Answer comprehension questions	yes
Design
Each class is responsible for a single piece of the program	no - see inline comments
Classes are loosely coupled	yes
Wave 1
List rooms	yes
Reserve a room for a given date range	yes
List reservations for a given date	yes
Calculate reservation price	yes
Invalid date range produces an error	no
Test coverage	some - see inline comment
Wave 2
View available rooms for a given date range	yes
Reserving a room that is not available produces an error	yes
Test coverage	some - see inline comment
Wave 3
Create a block of rooms	no
Check if a block has rooms	no
Reserve a room from a block	no
Test coverage	no
Additional Feedback

This is a good start, but I feel that there's a way to go yet. It seems to me that your design choice to use class variables and methods on Reservation to keep track of the list of reservations ended up hobbling you as your project grew larger and more complex. This made it difficult to get good test coverage, and difficult to move on to wave 3.

That being said, the specific method code you wrote is very clean and readable, and struggling against and learning from tough design choices was part of the goal of this project. Keep up the hard work, and if you want to sit down and talk about different ways of structuring things let one of us know.

[droberts-sea]

This would be a great place to include a check that the date range is valid.

[droberts-sea]

Using a class variable on Reservation to store the list of reservations is an interesting idea, and matches pretty closely what we did on Grocery Store. However, I think that in this case it ends up making the design more complicated.

The reason is that the Reservation class now has two responsibilities:

• Instances of Reservation must keep track of the details of an individual reservation
• The Reservation class itself must do all the work of keeping track of the list.

Having these two responsibilities tied together like this makes the code inflexible and difficult to extend, as you discovered when you began work on Wave 3.

It also makes it more difficult to test, because there's no easy way to reset all the state of a class.

An alternative approach would be to create another class, something like Hotel, that keeps track of all the rooms, reservations and blocks.

[droberts-sea]

If you're saving the check_in and check_out in @dates, you should not also be saving them as instance variables.

[droberts-sea]

I would argue that this bit of logic to check whether a reservation conflicts with a date range is just complex enough to warrant its own method. In fact, I think it would make sense to remove it from this method entirely, and instead write an instance method DateRange#overlap?(other), which takes another date range and returns the result of this check.

That would be a clean separation of responsibilities between Reservation and DateRange, making DateRange easier to reuse for something like a block of rooms, and it would also make this logic easier to test.

[droberts-sea]

Other cases I'd be interested in:

• What if there are no reservations? (it should return an empty list)
• Do reservations that don't overlap the date range affect the list? (they shouldn't)
  • Check the boundaries of what counts as an overlap. What about a reservation that ends on the checkin date, or starts on the checkout date?
• What if your date range spans several small reservations? (all of those reservations should be in the list)

[droberts-sea]

This is a good start, but I'd like to see more coverage around what rooms are considered available. Cases I'd be interested in:

• Not available:
  • Same dates
  • Overlaps in the front
  • Overlaps in the back
  • Completely contained
  • Completely containing
• Available:
  • Completely before
  • Completely after
  • Ends on the checkin date
  • Starts on the checkout date