Katricia - Edges - Hotel

[krsmith7]

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 made the decision to create a hash to store the rooms and corresponding dates. I also considered just using an array of Reservation instances or an Array of Rooms. I wanted each date to be connected with the dates reserved and with the corresponding reservations, so that led me to choose a hash of hashes structure.
Describe a concept that you gained more clarity on as you worked on this assignment.	I gained more clarity on loosening the dependencies by reducing the times I called classes or variables explicitly and passing in parameters instead. Though for one method or two methods, that may add a different element of note since the parameter can only be a very specific format, an instance variable of an instance of a class, which the user has to remember or realize from the output.
Describe a nominal test that you wrote for this assignment.	That for every reservation added, the length of the reservations array increases by one.
Describe an edge case test that you wrote for this assignment.	That a no vacancy message is returned if all rooms are reserved for a given date.
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	I think that this helped outline the program and process. It gave me more direction in what I should be writing the code to do. Sometimes writing the method after the test made me more quickly be able to correct the method or vice versa.

[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
Classes are loosely coupled	see inline
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 - missing some
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
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 - good work!
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	yes
Wrap Up
There is a refactors.txt file	yes
The file provides a roadmap to future changes	yes
Additional Feedback

Good work overall. Your design for how to keep track of reservations is a little odd but it gets the job done, and experimenting with design is one of the goals for this project. It is important to me that you understand why I find it odd, and that you're comfortable dividing up responsibilities between classes - as is, everything landed in BookingManager. It would be worthwhile to take a look at the instructor implementation of this project, and compare it with your own.

Outside of design, I like what I see. The code in your methods is generally well written. There are places where your test coverage could be expanded, but testing is a hard skill and takes a lot of practice to master. It would have been good to get to wave 3, but in terms of the fundamentals, it feels to me like you are in a good place. Keep up the hard work!

[droberts-sea]

This does exactly the same thing as attr_reader :reservations, just with a different name. In my mind, the attr_reader meets the requirements here, and is a little cleaner.

[droberts-sea]

The Reservation should have all the info about the number of nights and the cost per night, which indicates to me that this should be a method on the Reservation, not on the BookingManager. This is an example of not quite giving Reservation a whole responsibility (which in turn means BookingManager has more than one responsibility).

[droberts-sea]

I would probably not use a ternary here - it feels like too many things on one line.

[droberts-sea]

In general, it's considered bad practice to return a string in a failure case like this. One of the main reasons is that whoever calls this code will have to do a lot of work to determine whether or not it failed.

Instead you should either raise an exception, or (since this is supposed to return a collection) return an empty array. A simple implementation of the second option would be replacing line 143 with

return found_vacancies

[droberts-sea]

Since the start date and end date don't change inside this loop, it would be a little more efficient to build the date_range before the loop rather than inside it.

[droberts-sea]

You do a great job of using vertical whitespace (newlines) to break up the ideas in your code. This makes the whole program much easier to read.

[droberts-sea]

I like that you've broken each of these initialization helpers out as separate methods - the pattern is very clear.

[droberts-sea]

I like that you check the old number first here.

[droberts-sea]

There are a number of edge cases around when reservations overlap that you're missing here. A list was posted in slack at one point.

[droberts-sea]

I like this test! Good work covering this edge case.