forked from AdaGold/hotel
-
Notifications
You must be signed in to change notification settings - Fork 40
Time - Emily #42
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
Open
snowistaken
wants to merge
1
commit into
Ada-C13:master
Choose a base branch
from
snowistaken:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Time - Emily #42
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Binary file not shown.
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,104 +1,55 @@ | ||
| # Hotel | ||
|
|
||
| <!-- | ||
|
|
||
| ################ | ||
| NOTE ABOUT HOTEL | ||
| ################ | ||
|
|
||
| Section 1: Major Learning Goals | ||
|
|
||
| - Complete this section by skimming through the code and looking for "red flags." (For example, not having instance methods is a red flag. Having a class that is too big is a red flag.) | ||
|
|
||
| Section 2: Code Review and Testing Requirements | ||
|
|
||
| - Complete this section by skimming through the code and looking for the syntax that fulfills the requirement. If you can't find this requirement, then it's a no. These are written to be as yes/no as possible. | ||
|
|
||
| Section 3: Feature Requirements | ||
|
|
||
| - Complete this section by skimming through the test names, first. In the ideal world, you'd be able to find all of the answers by looking through the tests. Please note that each requirement is phrased as "There is a method that..." which is also ideal/likely for our students, but isn't guaranteed. | ||
|
|
||
| - Dee's note: I didn't add in all of the features that were required by the project; the project reqs not represented on this rubric are: | ||
| - Wave 1: Invalid date range produces an error | ||
| - Wave 2: Reserving a room that is not available produces an error | ||
| - Wave 3: Check if a block has rooms | ||
| --> | ||
|
|
||
|
|
||
| <!-- Instructors: The checkmarks are already there, so just delete them for any line items that aren't met. --> | ||
|
|
||
| ## Section 1: Major Learning Goals | ||
|
|
||
| <!-- Instructors: Feel free to practice creating specific feedback by referencing a line of code if you'd like. For example, you may say something like "nice custom method in `calculator.rb` line 42." This is optional. --> | ||
|
|
||
| | Criteria | yes/no, and optionally any details/lines of code to reference | | ||
| | --- | --- | | ||
| Practices SRP by having at least two separate classes with distinct responsibilities, and test files for these two classes | ✔️ | ||
| Overall, demonstrates understanding instance variables vs. local variables. (There aren't unnecessarily too many instance variables, when it should be a local variable) | ✔️ | ||
| For each test file, tests demonstrate an understanding of instantiating objects properly, and using Arrange-Act-Assert | ✔️ | ||
| Practices pseudocode and TDD, and reflected on it by filling out the reflection questions | ✔️ | ||
| Practices git with at least 15 small commits and meaningful commit messages | ✔️ | ||
|
|
||
| ## Section 2: Code Review and Testing Requirements | ||
|
|
||
| | Criteria | yes/no, and optionally any details/lines of code to reference | | ||
| | --- | --- | | ||
| There is a class that represents a reservation, and a second class that holds/manages a collection of reservations through composition (instance variable) | ✔️ | ||
| The logic for checking if a reservation's date overlaps with another reservation's date is complex logic that is separated into method(s) (and potentially class(es)) | ✔️ | ||
| The logic for checking if a reservation's date overlaps with another reservation's date has unit tests | ✔️ | ||
| All of the given tests run and pass | ✔️ | ||
| A test coverage tool is installed and used, and shows 95% test coverage | ✔️ | ||
|
|
||
| ## Section 3: Feature Requirements | ||
|
|
||
| | Feature Requirement: There is a method that... | yes/no | | ||
| | --- | --- | | ||
| gives back a list of rooms, and it's tested | ✔️ | ||
| creates a specific reservation for a room for a given date range, and it has nominal test cases | ✔️ | ||
| creates a specific reservation for a room for a given date range, and it tests an edge case, such as no available room, or invalid date range | ✔️ | ||
| gives back a list of reservations on a given date. Its tests include a test that makes several reservations for a given date | ✔️ | ||
| calculates the total price for a reservation | ✔️ | ||
| gives back a list of available rooms for a given date range, and it has nominal test cases | ✔️ | ||
| gives back a list of available rooms for a given date range, and it has edge test cases, such as no available room, or invalid date range | ✔️ | ||
| creates a block of rooms | ✔️ | ||
| reserves a room from a block | ✔️ | ||
| ## What We're Looking For | ||
|
|
||
| <!-- Reviewer Instructions: Note that canned positive feedback is included at the bottom - adjust this as appropriate to the submission. --> | ||
|
|
||
| ### Test Inspection | ||
|
|
||
| <!-- Reviewer Instructions: You should look through their tests to see if they are covering each of these cases. If they are not, check the functionality in pry or irb. If they have a good test: yes. If there is no test but it works in irb: yes but no test. If the test is failing, or the functionality doesn't work: no --> | ||
|
|
||
| Workflow | yes / yes but no test / no | ||
| --- | --- | ||
| **Wave 1** | | ||
| List rooms | | ||
| Reserve a room for a given date range | | ||
| Reserve a room (edge case) | | ||
| List reservations for a given date | | ||
| Calculate reservation price | | ||
| Invalid date range produces an error | | ||
| **Wave 2** | | ||
| View available rooms for a given date range | | ||
| Reserving a room that is not available produces an error | | ||
| **Wave 3** | | ||
| Create a block of rooms | | ||
| Check if a block has rooms | | ||
| Reserve a room from a block | | ||
|
|
||
| ### Code Review | ||
|
|
||
| **Baseline** | Feedback | ||
| --- | --- | ||
| Used git regularly | | ||
| Answer comprehension questions | | ||
| At least 95% test coverage | | ||
| **Design** | | ||
| Each class is responsible for a single piece of the program | | ||
| Classes are loosely coupled | | ||
| **Fundamentals** | | ||
| Names variables, classes and modules appropriately | | ||
| Understanding of variable scope - local vs instance | | ||
| Can create complex logical structures utilizing variables | | ||
| Appropriately uses methods to break down tasks into smaller simpler tasks | | ||
| Appropriately uses iterators and `Enumerable` methods | | ||
| Appropriately writes and utilizes classes | | ||
| Appropriately utilizes modules as a namespace | | ||
| **Wrap Up** | | ||
| There is a refactors.txt file | | ||
| The file provides a roadmap to future changes | | ||
|
|
||
| ## Overall Feedback | ||
|
|
||
| | Overall Feedback | Criteria | yes/no | | ||
| | --- | --- | --- | | ||
| | Green (Meets/Exceeds Standards) | 14+ total in all sections | | ||
| | Yellow (Approaches Standards) | 9-13 total in all sections | | ||
| | Red (Not at Standard) | 0-8 total in all sections, or assignment is breaking/doesn’t run with less than 5 minutes of debugging | | ||
|
|
||
| ### Additional Feedback | ||
|
|
||
| <!-- | ||
|
|
||
| ###### | ||
| PLEASE | ||
| ###### | ||
|
|
||
| Instructors, for Hotel, please give explicit positive feedback. | ||
|
|
||
| --> | ||
|
|
||
| Great 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... | ||
|
|
||
| I do see some room for improvement around... | ||
|
|
||
| ## Code Style Bonus Awards | ||
|
|
||
| <!-- Instructors: Please strike a balance between liberal/stingy with these. These are simply built-in pieces of positive feedback; use this to encourage and push students towards a cleaner code style! --> | ||
|
|
||
| Was the code particularly impressive in code style for any of these reasons (or more...?) | ||
|
|
||
| | Quality | Yes? | | ||
| | --- | --- | | ||
| | Perfect Indentation | ✅ | ||
| | Elegant/Clever | ✅ | ||
| | Descriptive/Readable | ✅ | ||
| | Concise | ✅ | ||
| | Logical/Organized | ✅ |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| require 'date' | ||
|
|
||
| require_relative 'reservation' | ||
| require_relative 'room' | ||
|
|
||
| module Hotel | ||
| class Block < Reservation | ||
|
|
||
| attr_reader :rooms, :rate | ||
|
|
||
| def initialize(rooms, stay_begin, stay_end, rate) | ||
| super(room, stay_begin, stay_end, rate) | ||
|
|
||
| @rooms = rooms.to_h { |room| [room, false]} | ||
| @rate = rate | ||
|
|
||
| claim_rooms | ||
| end | ||
|
|
||
| def any_available? | ||
| available_rooms = @rooms.select { |room, bool| bool == false} | ||
| if available_rooms.length > 0 | ||
| return true | ||
| else | ||
| return false | ||
| end | ||
| end | ||
|
|
||
| def reserve_room(first, last) | ||
|
|
||
| if first != @stay_begin || last != @stay_end | ||
| raise ArgumentError.new "#{first} - #{last} is not a valid date range!" | ||
| end | ||
|
|
||
| available_rooms = @rooms.reject { |room, bool| bool == true} | ||
| if available_rooms.length == 0 | ||
| raise ArgumentError.new("There are no rooms in the block available to book!") | ||
| else | ||
| @rooms[available_rooms.keys.first] = true | ||
| end | ||
|
|
||
| end | ||
|
|
||
| private | ||
|
|
||
| def claim_rooms | ||
| @rooms.each do |room, bool| | ||
| reservation = Hotel::Reservation.new(room, @stay_begin, @stay_end, @cost) | ||
| room.reservations << reservation | ||
| end | ||
| end | ||
|
|
||
| end | ||
| end |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| require 'date' | ||
|
|
||
| require_relative 'reservation_manager' | ||
|
|
||
| module Hotel | ||
| class Reservation | ||
|
|
||
| attr_reader :room, :stay_begin, :stay_end, :cost | ||
|
|
||
| def initialize(room, stay_begin, stay_end, cost = 200) | ||
| @room = room | ||
| @stay_begin = stay_begin | ||
| @stay_end = stay_end | ||
| @cost = (((stay_end - stay_begin).to_i / 24) - 1) * cost | ||
|
|
||
| verify_date(@stay_begin, @stay_end) | ||
| end | ||
|
|
||
| def overlapping?(first, last) | ||
| @stay_begin <= last && first <= @stay_end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def verify_date(stay_begin, stay_end) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Very cool that you made this a separate method! |
||
| if stay_begin - Date.today < 0 | ||
| raise ArgumentError.new("#{stay_begin} is before today. You cannot reserve a room for a past date.") | ||
| elsif stay_begin - stay_end > 0 | ||
| raise ArgumentError.new("#{stay_end} is before #{stay_end}. The end date must be after the begin date.") | ||
| end | ||
| end | ||
|
|
||
| end | ||
| end | ||
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| require 'date' | ||
|
|
||
| require_relative 'block' | ||
|
|
||
| # require_relative 'reservation' | ||
| # require_relative 'room' | ||
|
|
||
| module Hotel | ||
| class ReservationManager | ||
|
|
||
| attr_reader :rooms | ||
|
|
||
| def initialize | ||
| @rooms = create_rooms | ||
| end | ||
|
|
||
| def new_reservation(first, last) | ||
| available_rooms = @rooms.select { |room| room.available?(first, last)} | ||
| if available_rooms.length == 0 | ||
| raise ArgumentError.new "There are no available rooms to book!" | ||
| end | ||
|
|
||
| reservation = Reservation.new(available_rooms[0], first, last) | ||
| available_rooms[0].reservations << reservation | ||
| end | ||
|
|
||
| def new_block(rooms, block_begin, block_end, rate) | ||
| if rooms.length > 5 | ||
| raise ArgumentError.new("#{rooms.length} is more than the maximum number of allowed rooms for a block.") | ||
| end | ||
|
|
||
| rooms.each do |room| | ||
| if !room.available?(block_begin, block_end) | ||
| raise ArgumentError.new("Room #{room.number} is not available for the dates #{block_begin} - #{block_end}.") | ||
| end | ||
| end | ||
|
|
||
| block = Hotel::Block.new(rooms, block_begin, block_end, rate) | ||
|
|
||
| end | ||
|
|
||
| def reserve_block_room(block, stay_begin, stay_end) | ||
| block.reserve_room(stay_begin, stay_end) | ||
| end | ||
|
|
||
| def all_reservations_by_room(num, first, last) | ||
| room = @rooms.select { |room| room.number == num} | ||
|
|
||
| matching_reservations = room[0].reservations.select { |reservation| reservation.overlapping?(first, last)} | ||
|
|
||
| return matching_reservations | ||
| end | ||
|
|
||
| def all_reservations_by_date(first, last) | ||
| matching_reservations = [] | ||
|
|
||
| @rooms.each do |room| | ||
| matches = room.reservations.select { |reservation| reservation.overlapping?(first, last)} | ||
| matches.each do |match| | ||
| matching_reservations << match | ||
| end | ||
| end | ||
|
|
||
| return matching_reservations | ||
| end | ||
|
|
||
| def rooms_list | ||
| list = "" | ||
| @rooms.each do |room| | ||
| list += "Room Number: #{room.number}\n" | ||
| end | ||
| return list | ||
| end | ||
|
|
||
| def room_list_by_availability(first, last) | ||
| available_rooms = @rooms.select { |room| room.available?(first, last)} | ||
|
|
||
| return available_rooms | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def create_rooms | ||
| rooms = [] | ||
| 20.times do |i| | ||
| rooms << Hotel::Room.new(i + 1) | ||
| end | ||
| return rooms | ||
| end | ||
| end | ||
| end |
Oops, something went wrong.
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.
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 seems to cover every case and I actually can't believe it! I don't think anyone has written something so simple for this before, student or instructor.
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.
That said it would make me feel much more confident in this if there were thorough unit test coverage for it. I don't see any.