forked from AdaGold/hotel
-
Notifications
You must be signed in to change notification settings - Fork 46
Katrina - Edges - Hotel #39
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
kaganon
wants to merge
47
commits into
Ada-C10:master
Choose a base branch
from
kaganon: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
Changes from all commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
9b566a2
added room class and test to check if it creates and instance of room
kaganon 34f6bf7
updated Guardfile per Dan, and updated spec/spec_helper
kaganon 12c7fe7
Added methods to add rooms and list rooms
kaganon e455900
added initiliaze test for rooms list class
kaganon 4615f8e
removed room class and tests, do not need the room clas
kaganon 4848108
deleted rooms_list file and the spec file
kaganon cab2a88
Added Duration class that calculates nights booked
kaganon 53bcedd
added duration and room_number as instance variables
kaganon ff9f0c9
added list rooms method
kaganon ec586b7
created spec files for each class
kaganon 2e480d3
Added test for initializing reservation
kaganon 415878c
added test for calculates total cost of stay correctly
kaganon 29fe049
Fixed start_time and end_time - was not parsing correctly
kaganon e51bd13
added test to check if invalid dates raises an argument error
kaganon 9830b46
added is_overlapping method
kaganon c975f72
renamed Duration class to DateRange class
kaganon bf85cc5
moved check_valid_dates to the DateRange class
kaganon dd76228
updating class names to DateRange
kaganon c23a885
modified make_reservation method with new DateRange instance
kaganon 97cbd7c
restructured and moved around methods and tests
kaganon 9f94a4c
added and passed all tests for overlap dates method
kaganon a0ddc1d
added test for find_available_room method
kaganon b4aaf20
added test for list_reservations_by_date method
kaganon 3998e00
renamed RoomBooker class to BookingSystem class
kaganon daae5bf
modified all files that contained previous RoomBooker class to Bookin…
kaganon 404ba28
added a date_range parameter to the find_available_room method
kaganon bd5a54c
fixed logic on find_available_room method
kaganon 4d0e0c6
finally added correct available room to a reservation
kaganon 1cf82b4
added tests for correctly listing reservations by date
kaganon 84a19a2
added list available rooms method for a date range
kaganon ea0071f
added error handling for when booking a room that is not available
kaganon cb0bd73
added block room class
kaganon 45d390b
removed TODO notes and misc comments on files to clean up code
kaganon 058ea1b
added more tests to the BookingSystem class
kaganon f240dd9
added refactors.txt file
kaganon 318a5e7
added methods for making block room reservations
kaganon bf7a0c8
added method to find available rooms for all reservations including b…
kaganon f500703
added tests for reserving block rooms
kaganon c2ab0f4
finish comprehension questions
kaganon 3339c38
refactored logic for date_range
kaganon 705e444
removed list rooms method, redundant code
kaganon 79db25d
changed test for listing rooms using attr reader
kaganon 3452bfc
updated design-activity answers
kaganon 10c0f76
update header on design-activity
kaganon d77adda
format edits to design-activity
kaganon 88b2c8c
format table on design-activity
kaganon 81953e9
add more format changes to design-activity
kaganon 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
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
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,112 @@ | ||
| # Comprehension Questions | ||
|
|
||
| #### 1. What classes does each implementation include? Are the lists the same? | ||
| Both implementations include the same three classes, CartEntry, ShoppingCart, and Order. Yes, each implementation returns the same list in the Order class. The other classes however does not return the same list because Implementation B has instance methods that call on the `price` method for each class. Implementation A only has attributes in the other two classes. | ||
|
|
||
|
|
||
| #### 2. Write down a sentence to describe each class. | ||
| The CartEntry class creates an instance of an order item, which has the attributes unit price and quantity. In the second implementation, it contains the method price, which calculates the price of the item by multiplying the price of the item by the quantity of items ordered. | ||
|
|
||
| The ShoppingCart class creates an instance of a cart, which stores an array of cart item objects as an instance variable `@entries`. The second implementation contains the method price, which calculates the total of all cart items in the shopping cart. | ||
|
|
||
| The Order class creates an instance of an order, which stores an instance of a shopping cart object as an instance variable `@cart`. It calculates the total price of the order by getting the subtotal of the cart and adding the subtotal with the subtotal multiplied by the sales tax. | ||
|
|
||
|
|
||
|
|
||
| #### 3. How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper. | ||
| The Order class stores an instance of ShoppingCart object, which we can use to calculate the total price of a cart including the sales tax. Each ShoppingCart stores instances of CartEntry, and we can get the total price of the shopping cart by calling the method price on the cart (this method gets the subtotal of the cart without taxes). The CartEntry class stores an instance of a cart item which has the attributes unit price and quantity. We can get the price of a cart item entry by calling on the price method which multiplies the price of the item by the quantity ordered. | ||
|
|
||
| In other words, an Order has one shopping cart, a ShoppingCart has many cart entries, and a CartEntry has one entry item. | ||
|
|
||
|
|
||
| #### 4. What data does each class store? How (if at all) does this differ between the two implementations? | ||
|
|
||
| The CartEntry class stores a cart entry item. | ||
| The ShoppingCart class stores instances of CartEntry objects. | ||
| The Order class stores an instance of ShoppingCart. | ||
|
|
||
| Both implementations store the same data, but the behaviors of the classes differ. The second implementation wraps the attributes in an instance method, while the first implementation uses the attr_accessor to access the instance variables of the class. | ||
|
|
||
| #### 5. What methods does each class have? How (if at all) does this differ between the two implementations? | ||
|
|
||
|
|
||
| |**Implementation A** | CartEntry | ShoppingCart | Order | | ||
| | --- | --- | --- | --- | | ||
| | Methods | `initialize`, `attr_accessor` | `initialize`, `attr_accessor` | `initialize`, `total_price` | | ||
|
|
||
|
|
||
| | **Implementation B** | CartEntry | ShoppingCart | Order | | ||
| | --- | --- | --- | --- | | ||
| | Methods | `initialize`, `price` | `initialize`, `price` | `initialize`, `total_price` | | ||
|
|
||
| The two implementations differ in that the second implementation wraps the instance variables in an instance method and calls on that instance method to determine price in each class. On the other hand, the first implementation calls on the attr_accessor method from the other classes to determine the price in just one class, the Order class. | ||
|
|
||
|
|
||
| #### 6. Consider the Order `total_price method`. In each implementation: | ||
| * Is logic to compute the price delegated to "lower level" classes like ShoppingCart and CartEntry, or is it retained in Order? | ||
|
|
||
| - First implementation: price is retained in Order | ||
| - Second implementation: price is delegated to lower level classes, ShoppingCart and CartEntry | ||
|
|
||
| * Does total_price directly manipulate the instance variables of other classes? | ||
|
|
||
| - First implementation: directly manipulates the instance variables of other classes | ||
| - Second implementation: does not manipulate the instance variables, but uses the instance methods to get price | ||
|
|
||
|
|
||
| #### 7. If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify? | ||
| The second implementation is easier to change because we can add a conditional to the `price` method to lower the unit price if quantity is greater than or equal to a set amount. | ||
|
|
||
| The first implementation would be harder to change because all the logic is in the Order class, and is too dependent on the CartEntry and ShoppingCart classes. | ||
|
|
||
|
|
||
|
|
||
| #### 8. Which implementation better adheres to the single responsibility principle? | ||
| Implementation B better adheres to the single responsibility principle because each class has one responsibility and the class's dependencies on each other are minimal. They are loosely coupled, and implementing changes are easier compared to Implementation A. | ||
|
|
||
|
|
||
|
|
||
| #### 9. Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled? | ||
| Implementation B is more loosely coupled. | ||
|
|
||
|
|
||
|
|
||
| ## Improvements on Hotel Project | ||
|
|
||
| <!-- | ||
| What is this class's responsibility? | ||
| You should be able to describe it in a single sentence. | ||
| Is this class responsible for exactly one thing? | ||
| Does this class take on any responsibility that should be delegated to "lower level" classes? | ||
| Is there code in other classes that directly manipulates this class's instance variables? | ||
| You might recall writing a file called refactor.txt. Take a look at the refactor plans that you wrote, and consider the following: | ||
|
|
||
| How easy is it to follow your own instructions? | ||
| Do these refactors improve the clarity of your code? | ||
| Do you still agree with your previous assesment, or could your refactor be further improved? | ||
| --> | ||
|
|
||
|
|
||
| ### Notes on Class Responsibilities: | ||
| * Reservation: | ||
| - Responsible for holding one reservation instance | ||
| - BookingSystem calls on the reservation's instance variable `date_range`, but thought it was appropriate because `date_range` is an instance of class DateRange. This way, I can call on a method on DateRange and do the work there instead of in the BookingSystem class. | ||
|
|
||
| * DateRange: | ||
| - Responsible for validating available dates | ||
| - Instance variables are not directly manipulated, methods used to validate dates | ||
|
|
||
| * BookingSystem: | ||
| - Responsible for creating all reservations (room and blocks) | ||
| - Knows about all other class, but the other classes know little about each other | ||
|
|
||
| * BlockRoom: | ||
| - Responsible for holding a block of rooms with a discounted rate | ||
| - Like Reservation, the instance variable `date_range` in BlockRoom is called in BookingSystem | ||
|
|
||
|
|
||
| ### Hotel Refactors: | ||
| * Updated logic of the `dates_overlap` method in DateRange class. Helps clean up code/readability | ||
| * Created custom error exceptions within the DateRange and BookingSystem class, instead of raising StandardError | ||
| * Removed `list_rooms` method in the BookingSystem class since the method was redundant and attr_reader returns the same thing | ||
| * Fixed logic error in BookingSystem method `find_available_rooms` where there were nil inputs being populated in the array |
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,21 @@ | ||
| module Hotel | ||
| class BlockRoom | ||
| BLOCK_ROOM_RATE = 100.00 | ||
| attr_reader :date_range, :block_rooms | ||
|
|
||
| def initialize(date_range, block_rooms) | ||
| @date_range = date_range | ||
| @block_rooms = block_rooms | ||
| end | ||
|
|
||
| def block_dates_duration | ||
| return (@date_range.end_date - @date_range.start_date).to_i | ||
| end | ||
|
|
||
| def block_cost | ||
| return ("%.2f" % (block_dates_duration * BLOCK_ROOM_RATE)).to_f | ||
| 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,86 @@ | ||
| require_relative 'reservation' | ||
| require_relative 'block_room' | ||
| require_relative 'date_range' | ||
| require 'awesome_print' | ||
|
|
||
| module Hotel | ||
| class BookingSystem | ||
| class ValidateRoomsError < StandardError ; end | ||
|
|
||
| attr_reader :rooms, :reservations, :block_rooms | ||
|
|
||
| def initialize | ||
| @rooms = (1..20).to_a | ||
| @reservations = [] | ||
| @block_rooms = [] | ||
| end | ||
|
|
||
| def list_reservations(date_range) | ||
| return @reservations.select { |res| res.date_range == date_range } | ||
| end | ||
|
|
||
| def list_available_rooms(date_range) | ||
| return @rooms if @reservations.empty? | ||
| return find_available_rooms(date_range) | ||
| end | ||
|
|
||
| def find_available_rooms(date_range) | ||
| return @rooms if @reservations.empty? | ||
|
|
||
| unavailable_rooms = @reservations.map { | ||
| |res| res.room_number if date_range.dates_overlap?(res.date_range) | ||
| }.flatten.compact | ||
|
|
||
|
|
||
| unavailable_rooms_block = @block_rooms.map { | ||
| |res| res.block_rooms if date_range.dates_overlap?(res.date_range) | ||
| }.flatten.compact | ||
|
|
||
|
|
||
| all_unavailable_rooms = unavailable_rooms + unavailable_rooms_block | ||
|
|
||
| available_rooms = @rooms.reject { | ||
| |r| all_unavailable_rooms.include?(r) | ||
| } | ||
|
|
||
| raise ValidateRoomsError, "No rooms available for those dates." if available_rooms.empty? | ||
| return available_rooms | ||
| end | ||
|
|
||
| def make_reservation(date_range) | ||
| room = find_available_rooms(date_range).first | ||
| reservation = Reservation.new(date_range, room) | ||
| @reservations << reservation | ||
| return reservation | ||
| end | ||
|
|
||
| def reserve_block_rooms(date_range) | ||
| rooms = find_available_rooms(date_range).first(5) | ||
| block_reservation = BlockRoom.new(date_range, rooms) | ||
| @block_rooms << block_reservation | ||
| return block_reservation | ||
| end | ||
|
|
||
| def find_block(date_range) | ||
| raise ValidateRoomsError, "No block rooms found for those dates" if @block_rooms.empty? | ||
| return @block_rooms.select { |block| block.date_range == date_range } | ||
| end | ||
|
|
||
| def find_room_in_block(date_range) | ||
| found_block = find_block(date_range) | ||
| room_block_rooms = found_block.first.block_rooms | ||
| available_rooms = find_available_rooms(date_range) | ||
|
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. What if there's more than one block for a date range? |
||
| rooms = available_rooms.select { |room| room if room_block_rooms.include? available_rooms } | ||
| return rooms | ||
| end | ||
|
|
||
| def make_reservation_from_block(date_range) | ||
| room = find_room_in_block(date_range).first | ||
| reservation = Reservation.new(date_range, room) | ||
| @reservations << reservation | ||
| return reservation | ||
| 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,39 @@ | ||
| require 'date' | ||
|
|
||
| module Hotel | ||
| class DateRange | ||
| class ValidateDatesError < StandardError ; end | ||
|
|
||
| attr_reader :start_date, :end_date | ||
|
|
||
| def initialize(start_date, end_date) | ||
| @start_date = Date.parse(start_date) | ||
| @end_date = Date.parse(end_date) | ||
| end | ||
|
|
||
| def check_valid_dates | ||
|
|
||
| raise ValidateDatesError, "Invalid dates entered" if @start_date == nil || @end_date == nil | ||
|
|
||
| raise ValidateDatesError, "End date cannot be before start date" if (@end_date - @start_date) <= 0 | ||
| end | ||
|
|
||
| def dates_overlap?(date_range) | ||
|
|
||
| booked_dates = [*date_range.start_date..date_range.end_date] | ||
| new_dates = [*@start_date..@end_date] | ||
|
|
||
|
|
||
| if !(new_dates.first >= booked_dates.last || new_dates.last <= booked_dates.first) | ||
| # dates overlap | ||
| return true | ||
| elsif new_dates.first < booked_dates.first && new_dates.last >= booked_dates.last | ||
| # dates are containing | ||
| return true | ||
| end | ||
|
|
||
| return false | ||
| 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,21 @@ | ||
| module Hotel | ||
| class Reservation | ||
| PRICE_PER_NIGHT = 200.00 | ||
|
|
||
| attr_reader :date_range, :room_number | ||
|
|
||
| def initialize(date_range, room_number) | ||
| @date_range = date_range | ||
| @room_number = room_number | ||
| end | ||
|
|
||
| def number_of_nights | ||
| return (@date_range.end_date - @date_range.start_date).to_i | ||
| end | ||
|
|
||
| def total_cost | ||
| return ("%.2f" % (number_of_nights * PRICE_PER_NIGHT)).to_f | ||
| 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,5 @@ | ||
| Changes to make in the future: | ||
| - Change start_date and end_date to check_in and check_out | ||
| - If more attributes added to rooms (like different prices, suites, King vs Queen beds, etc), create a separate Room class. | ||
| - Refactor the dates_overlap? method | ||
| - Work on BlockRoom class/methods + Tests |
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,26 @@ | ||
| require_relative 'spec_helper' | ||
|
|
||
| describe 'BlockRoom class' do | ||
| describe 'Initializer' do | ||
|
|
||
| before do | ||
| @date_range_1 = Hotel::DateRange.new('2018-09-01', '2018-09-05') | ||
| @block_room = Hotel::BlockRoom.new(@date_range_1, [1,2,3,4,5]) | ||
| end | ||
|
|
||
| it 'Creates an instance of BlockRoom' do | ||
| expect(@block_room).must_be_kind_of Hotel::BlockRoom | ||
| end | ||
|
|
||
| it 'Takes date range as an instance of DateRange' do | ||
| expect(@block_room.date_range).must_be_kind_of Hotel::DateRange | ||
| end | ||
|
|
||
| it 'Takes block rooms as an array' do | ||
| expect(@block_room.block_rooms).must_equal [*1..5] | ||
| 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.
Good use of
selecthere.