Time - Yaz #37
Time - Yaz #37
Conversation
… and basic overlap? test
…and added case coverage to overlap?. Covered and tested all nominal and some edge cases.
…o throw error instead - wrote test and passed.
…for Room constructor and methods, all passed. Editted FrontDesk to reflect these changes - though FrontDesk is still not fleshed out.
…assing tests in front_desk_tests.rb. Note that scafolding for FrontDesk previously included a room id as a parameter to reserve_room - that is now gone. Also added private helper method in FrontDesk: find_room, which replaces the method get_room(id). Additionally, changed functionality of DateRange overlap? to be less complex.
…unctionality the same). Updated FrontDesk#reserve_room to reflect this change. Also changed Room#available? to use any? block instead of none? block. Added FrontDesk#get_avail_rooms(start,end) and thourough tests in front_desk_tests (all passing). Updated tests for Room to not trip on Argument error for unavailable room in the process of testing Room#get_reservations(start, end_date).
…en no rooms are available, and successfully reserves a room when all rooms have at least one reservation.
…pre-existing files
…meter rate(with default value 200) to Reservation, and changed error messages in FrontDesk and Room to be more specific.
…m). Wrote extensive tests in block_tests.rb.
…oved attr_reader and created a custom reader method that doesn't show user the nil values that booked rooms leave behind
…so that FrontDesk contains no direct references to Reservation - only through room. With this refactor, Room#add now has capability to create a reservation with the room_rate, if provided.
…nd unused variable warnings in block_test and front_desk_test
is this possible?
HotelSection 1: Major Learning Goals
Section 2: Code Review and Testing Requirements
Section 3: Feature Requirements
Overall Feedback
Additional FeedbackGreat work overall! You've built your first project with minimal starting code. This represents an incredible milestone in your journey, and you should be proud of yourself! I am particularly impressed by the way that you wrote extremely comprehensive tests. Furthermore, you've made design decision and written code that elegantly implements tricky logic. I've left a few in-line comment for you to review about ways you might consider refactoring. Overall it is clear that the learning goals around TDD and object oriented design were met. Keep up the hard work! Code Style Bonus AwardsWas the code particularly impressive in code style for any of these reasons (or more...?)
|
| @@ -0,0 +1,49 @@ | |||
| require 'date' | |||
| module Hotel | |||
| class InvalidDateError < StandardError | |||
|
|
||
| end | ||
|
|
||
| def overlap?(other) |
There was a problem hiding this comment.
I like your design choice to have Reservation and Block inherit from DateRange. I think I understand your logic here. Front_desk will invoke overlap? on instances of reservation with date_range as the argument, yea? Given that you're using the method in a child class (reservation), but other is a DateRange, you may consider adding a comment to explain this use case to increase readability. Also, correct me if I'm mis-understanding your logic.
| return reservations_at_date | ||
| end | ||
|
|
||
| private |
There was a problem hiding this comment.
Nice use of a private helper method.
| @@ -0,0 +1,109 @@ | |||
| require_relative "test_helper" | |||
|
|
|||
| describe Hotel::Block do | |||
There was a problem hiding this comment.
These tests are really comprehensive. Nice work!
| end | ||
|
|
||
| describe "nights" do | ||
| it "returns the correct number of nights" do |
There was a problem hiding this comment.
Wow. Comprehensive! I see you're testing different cases with each assertion here. You might consider splitting these up. For Instance
it "returns the correct number of nights when the reservation spans the new year" do
cont_start = Date.new(2016, 12, 29)
cont_end = Date.new(2017, 01, 01)
range = Hotel::DateRange.new(cont_start, cont_end)
expect(range.num_nights).must_equal 3
end
it "returns the correct number of nights when the reservation spans a change in month" do
cont_start = Date.new(2017, 01, 01)
cont_end = Date.new(2017, 02, 01)
range = Hotel::DateRange.new(cont_start, cont_end)
expect(range.num_nights).must_equal 31
end
| expect(rooms.any? {|room| room.reservations.include?(res5)}).must_equal false | ||
| expect(rooms.any? {|room| room.reservations.include?(res6)}).must_equal false | ||
| end | ||
|
|
There was a problem hiding this comment.
You check that reserve_room raises an Exception if there are no available rooms. Nice work! It's also important to test this edge case in available_rooms. What does available_rooms return when there are no available rooms?
| end | ||
| expect{@front_desk.reserve_room(Date.new(2020, 01,01), Date.new(2020,01,03))}.must_raise ArgumentError | ||
| end | ||
|
|
There was a problem hiding this comment.
Your block tests are quite comprehensive. Now that you have implemented blocks, you should also add a test here to make sure you can't reserve a room that's part of a block.
Assignment Submission: Hotel
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.
Reflection