-
Notifications
You must be signed in to change notification settings - Fork 49
Leaves - Cloudy #29
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?
Leaves - Cloudy #29
Conversation
…ze for date_range.rb and tests for date_range_tests.
…st, all passing, for it too.
…ses for it. boom boom.
…s. added a parameter to the hotel manager initalize to aid in this. added test for it. we good.
…dded date_of_res method too. test pass
…, added tests, passing.
…ure that block.rb works properly. added create block and reserve from block methods. test are all passing. added caveat to block initialize to adhere to the 5 block room max. test pass for that too.
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! I am particularly impressed by the way that you broke down the problem with methods while keeping the code loosely coupled. There is nothing but trim, direct code here. This code kicks tons of butt, and you should be super proud! I do see some room for improvement around editing your code to remove warnings. Also, don't forget to check your code coverage, it was including your tests, and therefore wasn't checking things properly. Your tests were thorough, but you need to prove it! |
| end | ||
|
|
||
| it "will raise an Argument Error if block requested is more than 5" do | ||
| expect{block = Hotel::Block.new(@checkin_date, @checkout_date, 100, [4, 5, 6, 7, 8, 9])}.must_raise ArgumentError |
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.
block isn't used, threw an warning
| require 'pry' | ||
| require 'awesome_print' | ||
|
|
||
| require_relative "../lib/reservation.rb" |
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.
circular require, warning
| require 'pry' | ||
| require 'awesome_print' | ||
|
|
||
| require_relative "../lib/date_range.rb" |
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.
circular require, warning
| require 'awesome_print' | ||
|
|
||
| require_relative "../lib/date_range.rb" | ||
| require_relative "../lib/hotel_manager.rb" |
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.
circular require, warning
| class Hotel_Manager | ||
| attr_reader :all_rooms, :available_rooms, :reservations,:block_reservations | ||
|
|
||
| def initialize(hotel_size) |
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.
The gist of the instructions tried to lead you to the conclusion that the hotel's size wasn't malleable. 20 rooms, always.
| end | ||
|
|
||
| avail_rooms -= rooms_blocked | ||
|
|
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 section could use some comments!
| avail_rooms = @all_rooms | ||
|
|
||
| block_overlaps = @block_reservations.select do |block| | ||
| block.overlaps?(date_range) |
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 work delegating!
|
|
||
| I really considered creating a csv to load from but, I dont think thats a possibility per the project. It couldve been fun(hard) to create and use it though I should definitely think that through a bit more because maybe the refactor wouldnt be worth the time? My tutor mentioned it early on and it did seem both easier and very straightforward and also intimidating bc I know I'm not super comfortable with reading (or writing) to csvs. | ||
|
|
||
| I should definitley go back and update the ArgumentErrors, I know I had to much fun with them. I see how giving myself a giggle would be a problem to someone else reviewing my code, especially if those errors were raised. |
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 was going to comment about this and then thought "nah, i did that all the time as a student." lol
Hotel
Congratulations! You're submitting your assignment!
Comprehension Questions