Katrina - Edges - Hotel

[kaganon]

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?	In the beginning of my design I had a room class, which contained the room numbers and cost. However, I ended up removing the room class because I realized I that I could just keep track of all the rooms in my BookingSystem class, and that the cost of the room would be stored as a constant variable in my Reservation class.
Describe a concept that you gained more clarity on as you worked on this assignment.	I think after drawing out what I needed from each class and seeing how the Booking System is the main driver code that connects all the other classes, it was a little bit clearer for me to figure out the direction of my program.
Describe a nominal test that you wrote for this assignment.	A nominal test in my program include correctly calculating total cost of a reservation. It tests to make sure that it calculates the total cost of the reservation, even when adding multiple dates to the same reservation.
Describe an edge case test that you wrote for this assignment.	An edge case I wrote included raising an error for invalid dates. For example if the user entered nil for either or both dates, it would raise an error.
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	I think it’s still challenging for me to think of pseudo code and tests first before writing code, especially if I’m not sure in my design of the code. But I’ve found that it’s been getting better with more practice.

[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
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	mostly
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	Great job overall! Your design is clean, your code well-written and your test coverage solid. I spotted at least one logic bug, and as always there are a few places where things could be cleaned up, but in general I am quite happy with what I see here. Keep up the hard work!

[droberts-sea]

This is doing the same thing as attr_reader :rooms, just with a different name. I would say that the attr_reader hits the project requirement, and you could omit this method.

[droberts-sea]

Good use of select here.

[droberts-sea]

This code has some problems. On line 31, you'll end up with a bunch of nils in your array, since map must always produce a value even if none of the code in the block executes.

On line 33, not only will you have nil, but the elements in your array are more arrays, since that's what a BlockRoom stores. That means that after line 35 has run, you'll have an array like:

[nil, 3, nil, nil, 5, nil, [8, 9, 10], nil]

A side effect of this is that your code does not respect blocks when making reservations!

You could use a combination of Array#flatten and Array#compact to fix it, or it could be that map isn't a great choice for this problem.

[droberts-sea]

I do love the way this code flows though - logic problems aside, this is an excellent example of loose coupling. Rather than pulling the dates out of the reservation and doing a bunch of math here, you call a method and let the date range take care of it. That makes this code easier to read, and it means if something about how overlaps are calculated changes this code won't have to. Good work!

[droberts-sea]

What if there's more than one block for a date range?

[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]

I like the way you've placed this logic on the DateRange class - good division of responsibilities.

However, this check can be done in a single line! I challenge you to figure out how. Or you can go read the instructor implementation.

[droberts-sea]

Good work catching this error case!

[droberts-sea]

Though the list of these checks may originally have come from the instructors, you did the work of interpreting them and implementing them as tests. I appreciate your diligence.