Pipes - Kimberley Zell - Hotel

[kimpossible1]

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?	When making Blocks I had to decide if I wanted to make a separate class for Block Admin or handle it under the same Admin area as the regular Bookings. I also had to decide if I wanted to make a separate class for Block Bookings. I went with making a class for the Block itself, and then using my Booking class when making a block booking. I kept the admin part with my Reservations class. The functionality of each part of the Block helped me decide where to put it.
Describe a concept that you gained more clarity on as you worked on this assignment.	I think I better understand what initialize does and what can be put in initialize. I also better understand what the attr reader vs accessor does. I had started using most att_reader from what we talked about in class, that it's important to not let things be write accessible if there isn't need for it, but I did not realize that would mean I also cannot access things that are not writeable in other parts of my code. So after I understood that better, I started using attr_accessor more. Overall I feel that I'm getting better with using classes and definitions.
Describe a nominal test that you wrote for this assignment.	I wrote a test to make sure my program won't allow a room booking to go through if the room is already booked.
Describe an edge case test that you wrote for this assignment.	I wrote a test to make sure that my program won't allow a booking on a room block if the room is not reserved for the block.
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	I think I did okay. I tend to sometimes write some code in with my pseudocode. I'm pretty sure my code could be refactored to be more concise, and there must be a better way to organize. The Reservations class became particularly long. I perhaps could make more classes.

[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	some - see my inline comments about DateRange
Classes are loosely coupled	mostly - I noticed a couple places where classes are hard to separate, but in general this looks good.
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	yes
Test coverage	missing some cases around listing reservations and available rooms - see inline
Wave 2
View available rooms for a given date range	yes
Reserving a room that is not available produces an error	yes
Test coverage	missing many interesting cases around availability checking - see inline
Wave 3
Create a block of rooms	yes
Check if a block has rooms	yes
Reserve a room from a block	yes
Test coverage	missing some
Additional Feedback
File Names	The ruby standard is that files should be named using snake_case - all lowercase, underscores between words. So instead of lib/DateRange.rb it would be lib/date_range.rb. Same for spec files, so the corresponding spec would be specs/date_range_spec.rb.
General Vibe	This is a good start, but it feels like there's a way to go still. The general pattern I notice is that your specific code within a method is usually solid, but design and test coverage could be improved. To me, this indicates you're having trouble taking a step back and imagining what your code looks like from the outside, thinking about what pieces ought to go where or what could go wrong. This important skill is difficult to master, but it's one you can grow with deliberate practice.

[droberts-sea]

This method will recreate all the rooms from scratch every time it's called. This is a problem because any reference to a room from before won't register as equal to a room from after even if they have the same room number, because the object IDs will be different.

A better strategy might be to check if the list already exists at the beginning of this method, and if it does immediately return it.

[droberts-sea]

All the places that you create a DateRange, it seems like you're only using it as a tool to get an array of dates. This indicates that a method might be a better choice here, one that takes a checkin and checkout date and returns an array of dates.

The other option would be to write an instance method, something like DateRange#overlap?(other), that takes another DateRange and checks to see if they overlap. Then this complex bit of logic would be hidden away in DateRange instead of sitting here in Reservations.

As long as you've identified "manage a range of dates" as a responsibility, then it probably makes sense to do the later, and keep that responsibility out of the Reservations class.

This would also make it much easier to test!

[droberts-sea]

Since you have this method defined here, you should not have an attr_accessor for :total_cost above (since this method will override the generated one).

[droberts-sea]

Instead of storing the check_in, check_out and dates here, it might be cleaner to only store the DateRange instance, and anyone who needs those attributes for a booking will have to go through date_range. That makes for a very clean separation of responsibilities: a Booking doesn't need to know what parts a DateRange has or how it works, it just knows that it has one.

[droberts-sea]

I like that you run this validation right in the constructor, and that you've broken it out into a separate method. It might make even more sense as a check on the DateRange class, rather than having a Booking be responsible for this.

[droberts-sea]

This is a good start, but I'd like to see more coverage around what date ranges will conflict with each other. 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

[droberts-sea]

Other cases I'd be interested in:

• What if there are no reservations? (it should return a list of all the rooms)
• 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 there are no vacancies? (it should return an empty array)
• What if your date range spans several small reservations? (all of the rooms from those reservations should be omitted from the list)

[droberts-sea]

You're missing some test cases here - they look very similar to the ones I would like to see for listing rooms by date.

[droberts-sea]

Good failure case!

[droberts-sea]

Similar missing cases as for Booking above.