Conversation
…antiation of reservation object
…d with room id, and update list of reservations; and tests associated with these three methods
…to its list of reservations. added a couple tests for this method that check a reservation was added and that it is of the correct type
…or an invalid date range
…f type Reservation
… a given date range; argumenterror clause if tries to reserve room unavailable on date
…nstance variable hotel_block when created
…ethod to add a block object to its array
… class. also created reserve hotel block method. added tests for these
… to reserve room or hotel block when room is set aside in hotel block
…el block. also now raises argument error if #rooms in a block is greater than 5
…ost correctly with discounted rate
HotelWhat We're Looking For
|
| def reserve_room(start_date, end_date, room_id, block_id = nil, discount = nil) | ||
| #check if room available during those days | ||
| rooms_available = available_rooms(start_date, end_date) | ||
| raise ArgumentError if !(rooms_available.include?(room_id)) && block_id == nil |
There was a problem hiding this comment.
What if instead of calling available_rooms and then checking for inclusion, you called find_room and then is_available? directly? That would save you calling is_available? on all the other rooms.
| def available_rooms(start_date, end_date) | ||
| avail_rooms = [] | ||
| rooms.each do |room| | ||
| avail_rooms << room.id if room.isAvailable?(start_date, end_date) |
There was a problem hiding this comment.
The .select enumerable might be helpful to clean up this code a little.
| def access_reservations(date) | ||
| list_reservations = @reservations.select { |reservation| (Range.new(reservation.start_date, reservation.end_date)).include?(date) } | ||
| return list_reservations.map { |reservation| reservation } | ||
| end |
There was a problem hiding this comment.
On line 45 you do a lot of work with the internals of the reservation. I know we hadn't read POODR ch 4 yet when you wrote this code, but this is exactly what Metz is talking about when she says you should "ask for what instead of telling how". A cleaner approach might be to add a .includes_date? method to reservation, then use it here:
return @reservations.select { |res| res.include_date?(date) }Also the map on line 46 is not needed.
| @block_id = block_id | ||
| @discount = discount | ||
| @total_cost = calc_total_cost | ||
| end |
There was a problem hiding this comment.
In stead of storing @total_cost in an instance variable, it might be wise to rename the calc_total_cost method total_cost. That way if the price, discount, or number of nights were ever to change, the total price would "update" automatically (update is in quotes because it was never stored in the first place).
| def check_date_range_conflict(start_date, end_date, array) | ||
| if array != [] | ||
| array.each do |item| | ||
| return true if (!(item.start_date...item.end_date).include?(start_date) && |
There was a problem hiding this comment.
I would ask for more descriptive variable names here than array and item. What do you expect these variables to contain?
| end | ||
| @hotel = Hotel::Hotel.new( | ||
| id: 1, | ||
| rooms: @rooms, |
There was a problem hiding this comment.
Rather than messing around with an entire Hotel in the Block tests, you should instantiate a Block directly and test on that.
|
|
||
| it "adds the reservation to the Hotel's list of reservations" do | ||
| new_reservation = @hotel.reserve_room(Date.new(2001, 3, 5), Date.new(2001, 3, 7), 0) | ||
| expect(@hotel.reservations.length).must_equal 1 |
There was a problem hiding this comment.
What happens if you try to reserve a room that's already taken?
| expect(@hotel.find_room(0)).must_be_kind_of Hotel::Room | ||
| end | ||
|
|
||
| it "returns the correct room id" do |
There was a problem hiding this comment.
I would probably combine these two tests into one. However there is a case you're missing: what if the room to be found doesn't exist?
| it "raises an Argument Error if tries to reserve room that is not available for a given day" do | ||
| expect { @hotel.reserve_room(Date.new(2001, 3, 5), Date.new(2001, 3, 17), 12) }.must_raise ArgumentError | ||
| end | ||
| end |
There was a problem hiding this comment.
Aha, here is the test I asked for above. Since this exercises the reserve_room method, it should be grouped with those tests.
| require "Date" | ||
| describe "Room class" do | ||
| describe "Room instantiation" do | ||
| before do |
There was a problem hiding this comment.
You should have some tests for the is_available? method.
2 participants
Hotel
Congratulations! You're submitting your assignment!
Comprehension Questions