Time- Mair H #33
Open
Time- Mair H #33
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ia require and require_relative test files needed to run the minitests
…t is an instance variable in the Reservation class while an instance method in the DateRange class. updated tests accordingly
…p some of the reservation_dates tests for the Reservation class
…s- FrontDesk test are yet to be written
…n to implement the Room class method and instance variable- work in progress. no new tests written for any of the new changes
…m and dates_unavailable to Room to add all room reservations and return an array of all the dates the room has been booked respectively. wrote and modified list_of_reservations(date) and list_of_available_rooms(start_date, end_date) respectively to FrontDesk.
… date range dates and the duration of the stay respectively
…and a method to add room's reservation to it's own list
…d that call on DateRange object. room is inputted as a string and not a Room instance
…eing reservations for a specific date, and list of rooms available given a date range- tests are written but code base needs refactoring to dry up the code
…ion given a start and end date
kaidamasaki
reviewed
Mar 17, 2020
| attr_reader :start_date, :end_date | ||
|
|
||
| def initialize(start_date, end_date) | ||
| raise(ArgumentError, "The end date #{end_date} is smaller or same as the start date #{start_date}.") unless end_date > start_date |
There was a problem hiding this comment.
Great use of a guard clause!
Comment on lines
+26
to
+30
| if (dates_unavailable & test_date_range).length > 0 | ||
| return false | ||
| else | ||
| return true | ||
| end |
There was a problem hiding this comment.
You can just directly return the condition here (once you negate it), since it's a boolean value:
Suggested change
| if (dates_unavailable & test_date_range).length > 0 | |
| return false | |
| else | |
| return true | |
| end | |
| return !((dates_unavailable & test_date_range).length > 0) |
Or even better:
Suggested change
| if (dates_unavailable & test_date_range).length > 0 | |
| return false | |
| else | |
| return true | |
| end | |
| return (dates_unavailable & test_date_range).length <= 0 |
Comment on lines
+69
to
+70
| before do | ||
| end |
There was a problem hiding this comment.
You don't need to include a before if you don't have anything to stick in it.
Comment on lines
+18
to
+19
| expect(@reserve.instance_variable_get(:@start_date)).must_equal @start_date | ||
| expect(@reserve.instance_variable_get(:@end_date)).must_equal @end_date |
There was a problem hiding this comment.
In the future don't use things like instance_variable_get instead just put an attr_reader on the class.
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 did your test setup. I liked that you made sure there were several useful variables that you could re-use in future tests. I do see some room for improvement around TDD as mentioned above and also the design topics you touched on in your refactors file. Code Style Bonus AwardsWas the code particularly impressive in code style for any of these reasons (or more...?)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Assignment Submission: Hotel
Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.
Reflection