-
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 - Elizabeth #43
base: master
Are you sure you want to change the base?
Conversation
…one I created a module called Booking and a class with the corresponding file name. Created test files for each of the classes. Require_relative the test_helper on each test file.
… them as _tests instead of _test. Wrote two tests in hotel_test and wrote the initialize in hotel.rb.
…ts. Wrote make_reservation method and wrote a test. Wrote a sectionn of make_reservation that I will implement later and a corresponding test to use at a later time when I have more of an idea of how the Reservation Class will work. All tests pass.
…Wrote total_nights in DateRange to calculate the reservations total nights. Wrote tests for total_nights. DateRange passes all tests.
… Everything passes.
…ilable. Wrote two tests for it that aren't working properly yet but will be when I integrate available_rooms into make_reservation.
…s available. Wrote a test and passed it.
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 found ways to break down bigger problems into small, readable methods I do see some room for improvement around delegating methods, and improving your test cases. |
test/hotel_block_test.rb
Outdated
|
||
it "creates an instance of Hotel_Block" do | ||
start_date = Date.new(2019, 10, 10) | ||
end_date = Date.new(2019 10, 15) |
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 errors out every time. Run a final rake before you commit code! Even if the test isn't complete or doing anything, never check in code that crashes.
test/hotel_block_test.rb
Outdated
date_range = Booking::DateRange.new(start_date, end_date) | ||
total_nights = 5 | ||
total_cost = 1000 | ||
wedding_block = Booking::Hotel_Block.new(date_range, total_nights, total_cost, room) |
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.
room does not exist. This doesn't cause a crash but is a pretty easy-to-fix error
def find_reservation(start_date, end_date) | ||
reservation_array = [] | ||
reservations.each do |reservation| | ||
if reservation.date_range.start_date == start_date && reservation.date_range.end_date == end_date |
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 reaching too far into your reservations and date ranges. This should be cleared up by the POODR chapter 4 discussion.
@room = room | ||
@total_nights = total_nights | ||
@total_cost = total_cost | ||
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.
If you have a class that has no methods besides initialize
, either the class should be a struct of some sort, or some other part of your code is doing its work for it.
@@ -0,0 +1,17 @@ | |||
require_relative 'hotel' |
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.
/Users/devin/Documents/Ada/c12/hotel/lib/reservation.rb:1: warning: loading in progress, circular require considered harmful - /Users/devin/Documents/Ada/c12/hotel/lib/hotel.rb
|
||
attr_reader :number, :cost | ||
|
||
def initialize (number, cost: 200) |
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 space causes this warning:
/Users/devin/Documents/Ada/c12/hotel/lib/room.rb:6: warning: parentheses after method name is interpreted as an argument list, not a decomposed argument
20.times do | ||
res_start_date = Date.new(2019, 2, 15) | ||
res_end_date = Date.new(2019, 2, 20) | ||
res = @the_grand_budapest_hotel.make_reservation(res_start_date, res_end_date) |
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.
/Users/devin/Documents/Ada/c12/hotel/test/hotel_test.rb:111: warning: assigned but unused variable - res
test/reservation_test.rb
Outdated
res_room = Booking::Room.new(5) | ||
res_total_nights = res_date_range.total_nights | ||
res_total_cost = res_total_nights * 200 | ||
reservation = Booking::Reservation.new(res_date_range, res_total_nights, res_total_cost, res_room) |
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.
/Users/devin/Documents/Ada/c12/hotel/test/reservation_test.rb:11: warning: assigned but unused variable - reservation
require 'date' | ||
|
||
def build_test_reservation | ||
start_date = Date.new(2019, 2, 15) |
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 seems like you meant it to be a before
block.
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.
perhaps a let:
…culator to the reservation class. Fixed tests so everything passes.
Hotel
Congratulations! You're submitting your assignment!
Comprehension Questions