Sockets - Jansen

[JansenMartin]

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
What was a design challenge that you encountered on this project?	I had a tendency to make a single class do too much. For example, I originally had my 'is_available?' method inside of Manager. After reading POODR chapter 3, though, I realized that functionality would be better suited for Rooms.
What was a design decision you made that changed over time over the project?	I originally had Rooms and Reservation mainly exist to make instances of themselves. Over time, though, I gradually added more functionality that was originally supposed to go under Manager (such as 'is_available?' for Rooms and 'total_cost' for Reservation).
What was a concept you gained clarity on, or a learning that you'd like to share?	While reading about class dependencies in POODR, I saw the benefit of having clear divides between what classes do and don't know. Too much overlap makes it more difficult to debug when you inevitably refactor your code.
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case?	My tests for the 'is_available?' method check a variety of check-in and check-out date combinations. Making sure a booking date that STARTS BEFORE and ENDS AFTER an existing reservation returns FALSE would be a nominal case, because there is a clear, obvious overlap between the two.
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case?	The 'validate_date' method in Reservation ensures that the user gives a viable date range. My tests cover what would happen if someone tries to enter the date ranges in MM/DD/YYYY or YYYY/DD/MM formats. It's an edge case because these formats could (and should) trigger errors since the code is written under the assumption that we're using a YYYY/MM/DD format.
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	When starting from scratch, this was extremely helpful in pushing past the initial block that comes from looking at a blank canvas. As I got deeper into the project, however, it was easier for me to forget to write my tests first. The pseudocode/tests/code pattern definitely made the beginning stages smoother than other parts of the project -- That's a lesson I'll carry with me in the future.

[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	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	yes
Test coverage	mostly - 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	yes
Wave 3
Create a block of rooms	N/A
Check if a block has rooms	N/A
Reserve a room from a block	N/A
Test coverage	N/A
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 Enumerable methods	yes
Appropriately writes and utilizes classes	yes
Appropriately utilizes modules as a namespace	yes
Wrap Up
There is a refactors.txt file	yes
The file provides a roadmap to future changes	yes
Additional Feedback	Great job overall! This code is well-organized, well-written and mostly well-tested. The design you ended up with does a good job of splitting responsibilities up into classes, and is easy to understand. I've left a number of inline comments below for you to review, but in general I like what I see. Keep up the hard work!

[droberts-sea]

Since we're inside the Hotel module already, you don't need the Hotel:: here.

[droberts-sea]

I'm not sure that returning an empty array is the right approach here. You might consider either raising an exception or returning nil.

[droberts-sea]

I think it might be possible to tighten up this code a little.

[droberts-sea]

This code shows me that you've done a good job breaking up functionality between classes. You could imagine having Manager do a bunch of date math here, but instead it asks the room whether it's available. This is what Metz is talking about when she says you should "ask for what instead of telling how".

[droberts-sea]

I like the decision to make this a method rather than storing the cost as an instance variable. That way if the number of nights or the cost per night were to change (say in a future version of the program), the total cost will automatically be correct.

[droberts-sea]

The .select enumerable might be helpful to clean up this code.

[droberts-sea]

This method has a bug! You do all this work in the each loop, but you don't return list at the end. So, you end up implicitly returning the result of .each, which is the original array.

$ pry -r ./lib/manager.rb
pry> m = Hotel::Manager.new
pry> m.reserve_room('2019-04-07', '2019-04-11')
pry> m.list_reservations_on('2020-01-01')
=> [#<Hotel::Reservation:0x00007feaa1922280
  @check_in=#<Date: 2019-04-07 ((2458581j,0s,0n),+0s,2299161j)>,
  @check_out=#<Date: 2019-04-11 ((2458585j,0s,0n),+0s,2299161j)>,
  @id=1,
  @room=#<Hotel::Room:0x00007feaa10dcd28>]

[droberts-sea]

This is something that your tests should have caught.

[droberts-sea]

What if you try to reserve without a specific room, and the whole hotel is booked?

[droberts-sea]

What about reservations that aren't associated with that date?

[droberts-sea]

Good work laying out all of these different test cases.

Since you recreate them so frequently, it might make sense to store the instances of Date as instance variables in the before block as well. Then in the tests, you could do something like:

date_range = @check_in_date..(@check_out_date - 2)