-
Notifications
You must be signed in to change notification settings - Fork 50
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
Leaves - Tiffany #25
base: master
Are you sure you want to change the base?
Leaves - Tiffany #25
Conversation
…tions overall. Created and passed all reservation initialization tests.
… passed for that item.
…lect changes to fix bugs.
…elblocks. not done yet.
…nputs to date object inputs.
HotelWhat We're Looking ForTest Inspection
Code Review
Overall 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! In particular nice work implementing Wave 3. Your use of inheritance for HotelBlock and the change_status method are quite clever. |
@@ -0,0 +1,2 @@ | |||
class FullOccupancyError < StandardError |
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 is a great example of defining your own appropriately named exception type.
|
||
# Returns only reservations and not hotel blocks as per specs. | ||
def list_reservations | ||
return @occupied.values.select { |value| value.class == Hotel::Reservation } |
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.
Ruby will be looking for names locally first, prioritizing local names, since they're all in the same module, there's no need to namespace Hotel::Reservation
. Reservation
should just work! Same with the rest of the namespacing in the code.
|
||
# Returns only reservations and not hotel blocks as per specs. | ||
def list_reservations | ||
return @occupied.values.select { |value| value.class == Hotel::Reservation } |
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.
clever use of select!
require_relative 'reservation' | ||
|
||
module Hotel | ||
class HotelBlock < Reservation |
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.
Great use of inheritance.
attr_reader :rooms, :reservations, :dates, :hotelblocks | ||
|
||
def initialize | ||
@rooms = Hotel::Room.generate_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.
Nice use of a class method.
def initialize | ||
@rooms = Hotel::Room.generate_rooms | ||
@reservations = [] | ||
@dates = [] |
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.
A comment to tell us what @dates
is meant to hold would be useful, as it's not immediately obvious.
expect(@first_res).must_be_instance_of Hotel::Reservation | ||
|
||
expect(@first_res.id).must_equal 1 | ||
expect(@first_res.start_date).must_equal ::Date.parse('2019-06-05') |
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.
Consider assigning dates to variables in your tests to make it clear what you are testing.
expect(@first_res.start_date).must_equal ::Date.parse('2019-06-05') | |
start_date = Date.parse('2019-06-05') | |
end_date = start_date + 3 |
Hotel
Congratulations! You're submitting your assignment!
Comprehension Questions