Conversation
HotelWhat We're Looking ForTest Inspection
Code Review
Overall FeedbackYou're off to a great start! 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! You've made good decisions about class structure and encapsulation of different functionality in different methods. You still need to work on putting it all together. You could start with some of the failing tests to clean up the functionality. SEe inline comments for suggestions on what's happening with each of your tests. You also need a few more tests to check for both positive and negative outcomes (when you're allowed to make a reservation and when you're not). |
test/test_helper.rb
Outdated
| require_relative "../lib/hotel_controller" | ||
| require_relative "../lib/reservation" | ||
| require_relative "../lib/date_range" | ||
| require_relative "../lib/room" |
There was a problem hiding this comment.
This last require_relative "../lib/room" should have been removed. I had to comment it out to make the tests run.
test/hotel_controller_test.rb
Outdated
| expect(new_range.given_range).must_be Hash | ||
| end | ||
|
|
||
| it "takes available rooms from unavailable hash and creates available array" do |
There was a problem hiding this comment.
Did you decide not to do this? If so, remove the test.
test/reservation_test.rb
Outdated
| @end_date = "2020-08-04" | ||
| @start_date = "2020-08-06" | ||
|
|
||
| new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil) |
There was a problem hiding this comment.
This test produces an error because this line appropriately raises an error:
| new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil) | |
| new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil) |
This line needs to be in the expect{} so that the test doesn't itself raise an error.
| new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil) | |
| expect{new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil)}.must_raise StandardError |
test/reservation_test.rb
Outdated
| selected_room = @room | ||
| new_reservation = Hotel::Reservation.new(@start_date, @end_date, selected_room) | ||
|
|
||
| new_reservation.nights = (@end_date) - (@start_date) |
There was a problem hiding this comment.
Nights doesn't have a writer method so you can not perform this assignment. This is evidenced by the test error
Hotel::Reservation::nights
/Users/becca/Documents/GitHub/c12-student-repos/hotel/test/reservation_test.rb:59: warning: instance variable @room not initialized
test_0002_calculates how many nights the guest is paying for ERROR (0.00s)
NoMethodError: NoMethodError: undefined method `nights=' for #<Hotel::Reservation:0x00007fca6c887d48>
Did you mean? nights
test/hotel_controller_test.rb
Outdated
| date_range = Hotel::HotelController.new | ||
| given_range = Hotel::DateRange.new(start_date, end_date) | ||
|
|
||
| expect(date_range.given_range).must_include date |
There was a problem hiding this comment.
Were you trying to provide date_range with given_range as an attribute? Your constructor for HotelController does not have anyway to assign as date_range:
def initialize
# @reservation
@date_range
@rooms = []
@reservations = []
test_0004_puts unavailable rooms into a hash ERROR (0.00s)
NoMethodError: NoMethodError: undefined method `given_range' for #<Hotel::HotelController:0x00007fca6c89d5a8>
/Users/becca/Documents/GitHub/c12-student-repos/hotel/test/hotel_controller_test.rb:167:in `block (3 levels) in <top (required)>'
test/hotel_controller_test.rb
Outdated
| end_date = Date.parse("2016/8/12") | ||
| selected_room = @room | ||
|
|
||
| new_reservation = Hotel::Reservation.new(start_date_new, end_date_new, selected_room) |
There was a problem hiding this comment.
This is a good test, but you have not defined start_date_new and end_date_new.
test_0003_ensures there is no overlap with dates ERROR (0.00s)
NameError: NameError: undefined local variable or method `start_date_new' for #<#<Class:0x00007fca6b1d0eb0>:0x00007fca6a931cf0>
Did you mean? start_date
/Users/becca/Documents/GitHub/c12-student-repos/hotel/test/hotel_controller_test.rb:155:in `block (3 levels) in <top (required)>'
test/reservation_test.rb
Outdated
|
|
||
| # start_date = Date.parse("2016/8/4") | ||
| # end_date = Date.parse("2016/8/6") | ||
| # selected_room = @room |
There was a problem hiding this comment.
Why is this code commented?
test_0002_checks the date is an instance of Date class FAIL (0.00s)
Expected nil to be an instance of Date, not NilClass.
/Users/becca/Documents/GitHub/c12-student-repos/hotel/test/reservation_test.rb:99:in `block (3 levels) in <top (required)>'
test/date_range_test.rb
Outdated
| date = Date.parse("2016/8/7") | ||
| new_range = Hotel::DateRange.new(start_date, end_date) | ||
|
|
||
| expect(new_range).must_include date |
There was a problem hiding this comment.
I believe must_include will only check if a value is in an array.
test_0001_determines whether a date is in a given range FAIL (0.00s)
Expected #<Hotel::DateRange:0x00007fca6c039c40 @start_date=#<Date: 2016-08-04 ((2457605j,0s,0n),+0s,2299161j)>, @end_date=#<Date: 2016-08-06 ((2457607j,0s,0n),+0s,2299161j)>> (Hotel::DateRange) to respond to #include?.
/Users/becca/Documents/GitHub/c12-student-repos/hotel/test/date_range_test.rb:23:in `block (3 levels) in <top (required)>'
lib/reservation.rb
Outdated
| # end | ||
|
|
||
| def in_date_range?(date) | ||
| return @date_range.in_date_range?(date) |
There was a problem hiding this comment.
Do you realize you have a recursive call here?
Hotel
Congratulations! You're submitting your assignment!
Comprehension Questions