Amber Lynn | Edges | Hotel Assignment

[griffifam]

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 considered making an actual DateRange class but chose against it feeling it was too late. I chose to make reservation a class, room a class, and a reservation_mngr a class because they all have their own attributes and behavior.
Describe a concept that you gained more clarity on as you worked on this assignment.	I need to have a better - clearer understanding of what data type I'm working on and using binding.pry to check values before writing code. I learned to check errors in my tests by looking over tests I already created that were similar and were passing and check against what I was doing wrong in later test.
Describe a nominal test that you wrote for this assignment.	Creating reservations - after creating 20 reservations the array I stored them in, length - should be 20.
Describe an edge case test that you wrote for this assignment.	Checking room_numbers - if room_number was zero, an argument error was raise
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	Writing psuedocode first was VERY helpful and a technique I actually look forward to applying in future assignments...like for real. I'd also incorporate in my psuedocode questions that I was to test as well.

[tildeee]

Hotel

What We're Looking For

Feature	Feedback
Baseline
Used git regularly	x
Answer comprehension questions	x
Design
Each class is responsible for a single piece of the program	x
Classes are loosely coupled	x
Wave 1
List rooms	x
Reserve a room for a given date range	x
List reservations for a given date
Calculate reservation price	x
Invalid date range produces an error	x
Test coverage	x
Wave 2
View available rooms for a given date range	Reservation_mgr's find_room method returns one available room (the first one)
Reserving a room that is not available produces an error
Test coverage	x
Wave 3
Create a block of rooms
Check if a block has rooms
Reserve a room from a block
Test coverage
Fundamentals
Names variables, classes and modules appropriately	You have a class named Reservation_mngr-- it is Ruby naming convention to name classes with TitleCase/CapitalCamelCase/PascalCase, aka: ReservationManager. Other stuff looks good
Understanding of variable scope - local vs instance	mostly, but in some of the tests, there are variables defined that aren't being used
Can create complex logical structures utilizing variables	x
Appropriately uses methods to break down tasks into smaller simpler tasks	could have practiced this more, but I see you made a helper method with Reservations calculate_cost method
Understands the differences between class and instance methods	x
Appropriately uses iterators and enumerables	At times used an each loop in order to find something by a specific id, which is also accomplished with Enumerable's find or detect methods (https://ruby-doc.org/core-2.5.1/Enumerable.html#method-i-find)
Appropriately writes and utilizes classes	x
Appropriately utilizes modules as a mechanism to encapsulate functionality	x
Wrap Up
There is a refactors.txt file	x
The file provides a roadmap to future changes	x
Additional Feedback

Hey Amber Lynn! I think you did a lot of good work on this project. In general, I am particularly impressed by the tests you have-- They're thorough and specific and cover all of the implementation code except for the find_room method!

Overall, I think that the design is on a good path: You have a Room to represent something that holds a room number and a price (per night?), a Reservation holds a bunch of data, and you have a Reservation Manager that keeps track of all of them, manages them, and has the logic to manage them and create them.

I think that a way to move forward with this project will be to rethink how Reservation Manager keeps track of reservations and the subtleties of how a (potential) reservation could conflict with another existing reservation. I think that the way your code uses this logic correctly (if you have an available room, make a reservation and shovel it into @reservations), but I think the logic is not quite there. I'm adding some comments here.

I'm making some comments on some other small things to think about, too!

In general, good work and set up to a large project here

[tildeee]

This constant isn't used anywhere in this class!

[tildeee]

Here, you call reservation.calculate_cost, which is calling the method calculate_cost on the instance of reservation. Reservation also holds and stores a cost value on it. I think that you could either use reservation.cost since the reservation holds cost on it with .cost, or you should remove the cost variable on reservation so it's not redundant!

[tildeee]

In these set of tests, you define @rez and don't use it anywhere in the tests-- so it's probably safe to get rid of @rez.

[tildeee]

This method gets called, but it doesn't change anything and the return value doesn't get stored anywhere! Do we need this line?

[tildeee]

@reservations stores all of the reservations made in the whole system. Why does it throw an error if there are more than 20?

For example, what if I made 20 non-conflicting reservations for different dates? Should there be an error thrown when I made the 21st reservation?

[tildeee]

You use @current_res_id to store the value of what the next reservation's reservation_id will be. It feels a little weird to me that Reservation_mngr hast to hold onto this, and has to have the logic to deal with this and update it and set it, since that feels kind of like a very brittle way to set Reservation ids. Is there a way we can remove this logic from Reservation_mngr? Also, does a reservation really need an id?

[tildeee]

I think that this conditional doesn't quite work. For example, assume the following things:
room is this:

#<Hotel::Room:0x00007f94d00d2728 @price=200, @room_number=1>

@reservations is this:

[#<Hotel::Reservation:0x00007f94d011bec8
  @check_in=#<Date: 2018-01-03 ((2458122j,0s,0n),+0s,2299161j)>,
  @check_out=#<Date: 2018-01-10 ((2458129j,0s,0n),+0s,2299161j)>,
  @cost=1400,
  @reservation_id=1,
  @room=#<Hotel::Room:0x00007f94d00d2728 @price=200, @room_number=1>>]

reservation is looking at the only entry in @reservations, so it is this:

#<Hotel::Reservation:0x00007f94d011bec8
 @check_in=#<Date: 2018-01-03 ((2458122j,0s,0n),+0s,2299161j)>,
 @check_out=#<Date: 2018-01-10 ((2458129j,0s,0n),+0s,2299161j)>,
 @cost=1400,
 @reservation_id=1,
 @room=#<Hotel::Room:0x00007f94d00d2728 @price=200, @room_number=1>>

the value of check_in is "01/03/2018"
the value of check_out is "01/10/2018"

...

Then the dates (check_in: 01/03/2018, checkout: 01/10/2018) and the reservation we're looking at (with check_in: 01/03/2018, checkout: 01/10/2018) should say that this room (room with id 1) is NOT available, but this conditional will evaluate as false, and therefore leave it rm_avail is true (and not go into the next line). This method will return the room with id 1 as the first available room.

[tildeee]

Even though the method create_reservation DOES call find_room within it, this test that is located in the describe block for "find_rooms method" doesn't call find_rooms in it... it'll be a better, more specific and targeted test if the test explicitly calls it.