-
Notifications
You must be signed in to change notification settings - Fork 40
Shonda Space #35
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
base: master
Are you sure you want to change the base?
Shonda Space #35
Conversation
Fixed simple cov Cleaned Resvation Class and Hotel Dispatch
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 broke down the problem and split the responsibilities evenly among all of the classes. Each class had a clear purpose, and was allowed to be "in charge" of its data in a really robust and meaningful way! Your work here is an excellent example of class delegation, one that you should be really proud of! I do see some room for improvement around cleaning up your code for submission. It's all working correctly, but leaving in long comments and dead, commented out code just adds noise when someone tries to understand your work. I also see you using a lot of comments, which isn't bad necessarily, but I think you might be able to trust your readers a little more, or (if the comments are more for you) come up with some naming conventions that help you classify different kinds of methods. Code Style Bonus AwardsWas the code particularly impressive in code style for any of these reasons (or more...?)
|
| class Reservation < Date_Range | ||
| attr_accessor :room_num, :check_in_date, :check_out_date | ||
|
|
||
| def initialize(room_num, check_in_date, check_out_date, hotel_block_reservation = false) | ||
| @room = room_num | ||
| @check_in_date = check_in_date | ||
| @check_out_date = check_out_date | ||
| @hotel_block_reservation = hotel_block_reservation | ||
| end | ||
|
|
||
| #each time the employee creates a reservation it is created here | ||
|
|
||
| # Every room is identical, and a room always costs $200/night | ||
| def total_cost_for_stay | ||
| number_of_nights? * 200 | ||
| end | ||
| end |
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.
Just for your notes, I would probably never write a class smaller than this. If you ever write a class that doesn't have any instance methods, it can probably just be a hash. :)
| #date range (5) 2001,2,8 - 2001, 2,10 2 nights | ||
| #date range (6) 2001,2,8 - 2001, 2,12 4 nights | ||
| #date range (7) 2001,2,12 - 2001,2,14 4 nights | ||
|
|
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.
These tests are excellent! Nice work!
| def check_overlap_with_room_reservations(requested_date_range) | ||
| # check if the check_in and check_out date overlaps with any other reservations for the room. | ||
| # Return true if no reservation conflicts with the checkin and checkout date. | ||
| if !@reservations.any? { |res| res.overlaps_in_reservations?(requested_date_range) } | ||
| return true | ||
| end | ||
| return false | ||
| end | ||
|
|
||
| def create_new_reservation(check_in_date, check_out_date, hotel_block_reservation = false) | ||
| new_res = Reservation.new(room_num, check_in_date, check_out_date, hotel_block_reservation) | ||
| @reservations << new_res | ||
| return new_res | ||
| end |
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.
I appreciate how you use Room to take some functionality away from the HotelDispatcher class here.
| # def contains(date) | ||
| # if date >= @check_in_date && date < @check_out_date | ||
| # return true | ||
| # else | ||
| # return false | ||
| # end | ||
| # end |
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.
Don't forget to pull out dead code before you submit!
| # This method is used to create a Hotel Block | ||
| # Takes in Room Ids and check in and check out date | ||
| # inside the method we create a method we create a variable called new requested_date_range which uses the Date_Range class | ||
| # room_ids is the number of rooms | ||
| # need to raise error if more than 5 rooms | ||
| # using the make rooms we |
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.
Unless this method is doing something really tricky or clever, I'd try to shorten these kinds of comment to 2 or fewer lines. That way I can see this method in the context of the other methods, which can help folks understand how they come together as a class.
| end | ||
| end | ||
|
|
||
| describe "Can create a Hotel room block??????" do |
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.
Can it?????? (Tests seem to say, "yes" :P)
Assignment Submission: Hotel
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.
Reflection