-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LJ Hotel #33
base: master
Are you sure you want to change the base?
LJ Hotel #33
Conversation
…n't figure out tests for both no method error
…_dates to attr_reader in Reservation - duh - tests are built and now working
…n#get_reservation_total
…tion#methods to BookingSystem for better handling
…ering Reservation overhaul
…o booking_system_spec - tests pass
…class methods, added room csv
…added test and BlockReservation class
HotelWhat We're Looking For
|
lib/room.rb
Outdated
module Hotel | ||
class Room | ||
attr_reader(:id) | ||
attr_accessor(:cost) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really want cost
to be an accessor?
lib/room.rb
Outdated
|
||
# method to find a room | ||
def self.find_room(id) | ||
@id.find { |room| room.id == id } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method doesn't make much sense because your self.all
method doesn't initialize an @@id
class variable, and in your @id
instance variable the instance variable is just a number not a list.
(rsv_start...rsv_end).to_a.map { |day| day.to_s } | ||
end | ||
|
||
def get_total_cost(included_rooms, rsv_start, rsv_end, status) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could do this method with the instance variables instead of using parameters.
Also it's neat how you have a Reservation which can have multiple rooms.
reservation.booked_dates.include? search_date.to_s | ||
end | ||
# search through block id's if not found in standard reservations | ||
if found_reservations.empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do this if it doesn't find reservations during that date. Instead you would always want to find both.
lib/booking_system.rb
Outdated
end | ||
end | ||
|
||
found_reservations = 0 if found_reservations.empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why return 0
if no reservations? Why not return an empty list?
end | ||
|
||
# method to return total cost based on reservation id | ||
def get_reservation_total(id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method seems to duplicate existing functionality.
unavailable_rooms << rsv.included_rooms | ||
end | ||
end | ||
available_rooms = all_rooms_arr - unavailable_rooms.flatten |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
super(input) | ||
raise ArgumentError, 'Invalid name' if input[:group_name].nil? || input[:group_name].empty? | ||
raise ArgumentError, 'Invalid # of rooms' if input[:room_qty].to_i < 2 || input[:room_qty].to_i > 5 | ||
@included_rooms = input[:included_rooms] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should also be able to make reservations from within a block, after the block is "held."
|
||
def make_a_reservation(guest, check_in, check_out, number_of_rooms = 1, status = :BASE) | ||
raise ArgumentError, 'Too many rooms' if number_of_rooms > 20 || !(number_of_rooms.is_a? Integer) | ||
open_rooms = find_available_rooms(check_in, check_out).first(number_of_rooms) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if there are not enough available rooms?
it 'can find available rooms given a check in and check out date' do | ||
check_in = "October 23 2019" | ||
check_out = "October 24 2019" | ||
expect(admin.find_available_rooms(check_in, check_out)).must_be_kind_of Array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could also check and verify each instance is a Room
Hotel
Congratulations! You're submitting your assignment!
Comprehension Questions