-
Notifications
You must be signed in to change notification settings - Fork 40
Space - Antonia #41
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?
Space - Antonia #41
Conversation
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 way that you broke down the problem into thoughtful methods, and used your methods to separate concerns within your project. I also like how you approached Rooms, as there isn't much data that concerns a room except the number! As a matter of fact, that's how instructors have approached this problem in the past! I do see some room for improvement around breaking down complex logic, and double checking the validity of tests. When we tackle problems like these, we want to make sure we are really thoroughly understanding the scope and nature of the problem. These are the warning that got printed out when I ran your tests Code Style Bonus AwardsWas the code particularly impressive in code style for any of these reasons (or more...?)
|
| if @start_date >= range.end_date || @end_date <= range.start_date | ||
| return false | ||
| else | ||
| return true | ||
| 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.
This is good start, but you are missing a few important cases with this logic! In situations like this, I try drawing a picture where I can visualize all the possibilities for input data.
| it "returns false" do | ||
| start_date = @range.end_date | ||
| end_date = @range.end_date | ||
| test_range = Hotel::DateRange.new(start_date, end_date) | ||
|
|
||
| expect(@range.overlap?(test_range)).must_equal false |
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 test is failing, and I'm pretty sure it's failing because of line 11 in the DateRange class.
| reservation_list = @hotel_controller.reservations(@date) | ||
|
|
||
| expect(reservation_list).must_be_kind_of Array | ||
| reservation_list.each do |res| | ||
| res.must_be_kind_of Reservation | ||
| 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 don't see you putting any reservations in this list, so this test ends up passing without actually checking that the code works, if you get my meaning.
Assignment Submission: Hotel
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.
Reflection