Val - Edges - Hotel

[valgidzi]

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?	One of the biggest design decisions I made was to not make a Room class. I decided to store the room numbers as hash keys paired with reservation date array values. I decided not to make a room class because in this program rooms only have data, not behavior.
Describe a concept that you gained more clarity on as you worked on this assignment.	I gained a better understanding of the importance of writing code that is easy to change. It was easier to make design decisions because I wasn't forced to stick with them if I decided to make another change down the road.
Describe a nominal test that you wrote for this assignment.	For the Reservation#get_all_dates method, I wrote a nominal test - "contains all dates for a reservation in the same month".
Describe an edge case test that you wrote for this assignment.	For the Reservation#get_all_dates method, I wrote an edge case test - "contains all dates for a reservation spanning 2 years".
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	Generally, I feel I did well with the workflow. Sometimes I would slip out of writing pseudocode and start writing actual code, but overall I followed the workflow and felt it gave me a better approach to writing my code.

[droberts-sea]

Hotel

What We're Looking For

Feature	Feedback
Baseline
Used git regularly	yes
Answer comprehension questions	yes
Design
Each class is responsible for a single piece of the program	yes
Classes are loosely coupled	see inline comments
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	yes
Wave 2
View available rooms for a given date range	yes
Reserving a room that is not available produces an error	yes
Test coverage	yes
Wave 3
Create a block of rooms	yes
Check if a block has rooms	yes
Reserve a room from a block	yes
Test coverage	yes
Fundamentals
Names variables, classes and modules appropriately	yes
Understanding of variable scope - local vs instance	yes
Can create complex logical structures utilizing variables	yes
Appropriately uses methods to break down tasks into smaller simpler tasks	yes
Understands the differences between class and instance methods	yes
Appropriately uses iterators and enumerables	yes
Appropriately writes and utilizes classes	yes
Appropriately utilizes modules as a mechanism to encapsulate functionality	no, but it looks like you're aware that this was missed
Wrap Up
There is a refactors.txt file	yes
The file provides a roadmap to future changes	yes
Additional Feedback

Great work overall!

I really like the way you split the booking manager and the calendar into two separate classes. This split isn't quite as clean as it could be, but the idea of it makes a lot of sense.

I do want to call out the way your code uses the Reservation class. You expect whoever is calling this code to do the work of creating a Reservation, and then you assign a room and add it to the calendar. There are a couple of issues with this.

1. While you assign dates to a room, you neither keep track of the Reservation itself nor assign a room to it, which means that when I call the add_reservation method I have to keep track of the room number separately.
2. I see that you're injecting the Reservation dependency into this method. I like the idea but in this case I'm not sure it's warranted. Another approach is to make this method to act like a factory, taking two dates and doing the work of creating a Reservation and assigning it a room, which is then returned to the caller. Ultimately the question is "who should know about the Reservation class", this code, or the code that calls it.

That being said, I'm quite happy with what I see here - you're clearly exploring different techniques of object-oriented design, which is the big goal for this assignment. The specific code you're writing and your tests are very solid. Keep up the hard work!

[droberts-sea]

I would probably put this check in the constructor, rather than in this other method. That makes it more obvious when the check happens, and what it means for a reservation to be invalid.

[droberts-sea]

Instead of storing number_of_nights in an instance variable and having an attr_reader, I would probably keep it as a method. The program will have to do slightly more work whenever you need the number, but it means that if you change the dates on the reservation the number of nights will update automatically. From the outside it won't look any different (you call a method and get back a number), but it makes this class more resilient to change.

[droberts-sea]

Good use of enumerables here.

[droberts-sea]

Instead of raising a StandardError, best practice is to define your own appropriately named exception type that inherits from StandardError and raise that. The reason is that StandardError is very broad, and if the code that calls this method has to rescue StandardError, that will catch things like ArgumentErrors and NameErrors that indicate true bugs.

[droberts-sea]

It might make sense to store the mapping from number of nights to discount in a hash. That way if you needed the info elsewhere in the program it would be relatively easy to access.

[droberts-sea]

This is an example of tight coupling. Instead of the BookingManager directly touching the Calendar's list of dates, it should call a method on the calendar (something like add_reservation), and the calendar should do that work.

That way if the calendar's internal system of keeping track of rooms and dates changes, this code won't have to.

[droberts-sea]

You could also say .first(block_size)

[droberts-sea]

Two more examples of tight coupling, both with the calendar and with the block

[droberts-sea]

Good work catching this edge-case

[droberts-sea]

Good use of dependency injection.