forked from AdaGold/hotel
-
Notifications
You must be signed in to change notification settings - Fork 45
Isabel Suchanek -- Carets #25
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
isabeldepapel
wants to merge
26
commits into
Ada-C8:master
Choose a base branch
from
isabeldepapel: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
26 commits
Select commit
Hold shift + click to select a range
c90c20d
Set up project and test files
isabeldepapel 5921835
Set up initialize and self.all (and tests) for Room class
isabeldepapel 7152f16
Initialized Reservation class (with tests)
isabeldepapel 42d4a85
Added reserve method to Room class
isabeldepapel b7550c5
Added hotel class
isabeldepapel 2fca0c9
Implemented all_reservations metho in hotel and added more error chec…
isabeldepapel 3709c03
Finished wave 1
isabeldepapel f788ac1
Commented out exceptions focused on input type instead of business logic
isabeldepapel 25dc256
Added find available rooms for date range method
isabeldepapel a89bc7e
Added is_booked method to Room class
isabeldepapel 13531e4
Refactored find_avail_rooms method
isabeldepapel 6f52656
Finished wave 2
isabeldepapel 9fe74cf
Set up Block class
isabeldepapel 75ff9a6
Added rate attribute to Reservation class
isabeldepapel c8e3547
Added reservable module
isabeldepapel 432323b
Added methods to find available rooms in Block class
isabeldepapel f20a7d0
Added reserve method in Block and modified Room.reserve to return Res…
isabeldepapel 61f4fd3
Finished wave 3, code needs cleaned up
isabeldepapel cdbe45d
Refactored is_booked and is_blocked methods
isabeldepapel ab468cd
Added discounted_cost to Block class and cleaned up some code
isabeldepapel d03b7a1
Refactored room tests and reservable.rb
isabeldepapel 88e3857
Added self.all and self.find methods to Block, Resv, and Room classes
isabeldepapel 815ac80
Refactored to use self methods and class vars
isabeldepapel dcc4b06
Added edge case to reserve test
isabeldepapel e7c017a
Added design activity questions
isabeldepapel 92ab683
Loosened coupling between Hotel & Room classes
isabeldepapel 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| require 'rake/testtask' | ||
|
|
||
| Rake::TestTask.new do |t| | ||
| t.libs = ["lib"] | ||
| t.warning = false | ||
| t.test_files = FileList['specs/*_spec.rb'] | ||
| end | ||
|
|
||
| task default: :test |
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,55 @@ | ||
| ## Prompts | ||
|
|
||
| 1. **What classes does each implementation include? Are the lists the same?** | ||
|
|
||
| Each implementation includes the same classes: | ||
| + a CartEntry class | ||
| + a ShoppingCart class | ||
| + an Order class | ||
|
|
||
| 2. **Write down a sentence to describe each class.** | ||
|
|
||
| + `CartEntry`: This is a single kind of item in the online shopping cart, and it tracks the unit price of the item, and the quantity of that item ordered. | ||
| + `ShoppingCart`: This is an object representing the actual shopping cart, and it consists of an array of entries (items in the shopping cart). | ||
| + `Order`: This is the online order, which contains a shopping cart, and it uses sales tax to calculate the total price of the cart contents. | ||
|
|
||
| 3. **How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper.** | ||
|
|
||
| A `ShoppingCart` has a `CartEntry`, and an `Order` has a `ShoppingCart`--i.e. `Order` depends on `ShoppingCart`, which depends on `CartEntry`. | ||
|
|
||
| 4. **What _data_ does each class store? How (if at all) does this differ between the two implementations?** | ||
|
|
||
| The classes in each implementation store the exact same data. `CartEntry` stores `unit_price` and `quantity`, `ShoppingCart` stores `entries` (which are cart entries), and `Order` stores `SALES_TAX` (a constant) and a cart, which is a `ShoppingCart` object. | ||
|
|
||
| 5. **What _methods_ does each class have? How (if at all) does this differ between the two implementations?** | ||
|
|
||
| Aside from the accessor methods, Implementation A only has one defined method, which calculates the total price of an order, which means it's more tightly coupled to both the `ShoppingCart` and `CartEntry` classes, because it depends on `ShoppingCart` being an array of CartEntries, and each `CartEntry` having a `unit_price` and `quantity`. | ||
|
|
||
| Implementation B, on the other hand, uses methods in both the `CartEntry` and `ShoppingCart` classes to calculate the price of each `CartEntry` object and the price of each `ShoppingCart` object. This decouples `ShoppingCart` from `CartEntry` (it calls `CartEntry`'s price method rather than manipulating `CartEntry`'s variables directly), as well as decoupling `Order` from both `ShoppingCart` and `CartEntry`. | ||
|
|
||
| 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`? | ||
| + Does `total_price` directly manipulate the instance variables of other classes? | ||
|
|
||
| + Implementation A: (see above) No, the logic to compute the price isn't delegated to the lower-level classes. Instead, `total_price` directly manipulates the instance variables of both `ShoppingCart` (it gets `@entries` from `ShoppingCart`) and `CartEntry` (for each `CartEntry`, it gets `@unit_price` and `@quantity`). | ||
|
|
||
| + Implementation B: (see above) Yes, the logic is delegated to the lower-level classes. `CartEntry` is responsible for calculating its own price, and `ShoppingCart` is responsible for calculating the total price of its contents. `Order` then simply calls the price method for `ShoppingCart` and adds sales tax to calculate the total price. | ||
|
|
||
| 7. **If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify?** | ||
|
|
||
| This would mean changing the `CartEntry` class so that if quantity exceeds a particular amount, `unit_price` is then set to a discounted rate. Implementation B is easier to modify, because it already has a `price` method, so it simply means modifying that `price` method to account for bulk orders. However, Implementation A has no such method—all the calculations are taken care of in the `Order` class, which means the `total_price` method will become more complex. | ||
|
|
||
| 8. **Which implementation better adheres to the single responsibility principle?** | ||
|
|
||
| Implementation B because each class calculates its own price using its own instance variables. | ||
|
|
||
| 9. **Bonus question: Which implementation is more loosely coupled?** | ||
|
|
||
| Implementation B | ||
|
|
||
|
|
||
| ## Refactoring Hotel | ||
|
|
||
| My `Hotel` class is pretty tightly coupled to the `Room` class, because it requires the user to pass `Room` objects (as opposed to a room number) to reserve or block rooms. To change this, I would make use of the hash I set up when initializing my `Hotel` class with rooms where the room number is the key and `Room` object is the value. Then, given a room number, I could first retrieve the `Room` object and then reserve it. This would be an improvement because it's much more user friendly (what user is going to want to find a room object before reserving it? No one, that's who). | ||
|
|
||
| Also, while the `Block` class can reserve rooms from within a block, that functionality isn't in the `Hotel` class itself, and it seems like it would make more sense to have all the blocking/reserving functionality of a hotel available in one class (provided that it delegates the appropriate tasks to lower-level classes), especially since you can reserve a room in the `Hotel` class so it doesn't make sense that you can't reserve a blocked room, as well. So I could have a method that calls the appropriate method(s) in the `Block` class to reserve a room in a given block. This is an improvement b/c it's also more user friendly and puts all the user interface methods (i.e. what the CLI would draw from) in one place rather than having them scattered around different classes. |
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,77 @@ | ||
| require_relative 'reservation' | ||
| require_relative 'room' | ||
| require_relative 'reservable' | ||
|
|
||
| module Hotel | ||
|
|
||
| class Block | ||
| include Reservable | ||
|
|
||
| @@all_blocks = [] | ||
|
|
||
| MAX_ROOMS = 5 | ||
|
|
||
| attr_accessor :check_in, :check_out, :discount, :room_block, :block_id | ||
|
|
||
| def initialize(check_in, check_out, discount, room_block) | ||
| # check input | ||
| check_dates(check_in, check_out) | ||
| check_room_block(room_block, check_in, check_out) | ||
| check_discount(discount) | ||
|
|
||
| @block_id = @@all_blocks.length + 1 | ||
| @check_in = check_in | ||
| @check_out = check_out | ||
|
|
||
| @discount = discount # discount as decimal (e.g. 0.8 for 80% of orig rate) | ||
| @room_block = room_block # array of rooms | ||
|
|
||
| # add block to each room | ||
| room_block.cycle(1) { |room| room.blocks << self } | ||
|
|
||
| @@all_blocks << self | ||
|
|
||
| end | ||
|
|
||
| def room_available?(room) | ||
| raise ArgumentError.new("Room #{room.room_num} isn't in the block") if !room_block.include?(room) | ||
|
|
||
| return !room.booked?(check_in, check_out) | ||
| end | ||
|
|
||
| def find_avail_in_block | ||
| return room_block.select { |room| room_available?(room) } | ||
| end | ||
|
|
||
| def reserve(room) | ||
| raise ArgumentError.new("Room #{room.room_num} isn't in the block") if !room_block.include?(room) | ||
|
|
||
| raise ArgumentError.new("Room #{room.room_num} isn't available for the selected dates") if room.booked?(check_in, check_out) | ||
|
|
||
| discounted_rate = discount * room.rate | ||
| return room.reserve(check_in, check_out, discounted_rate) | ||
| end | ||
|
|
||
| def self.all | ||
| return @@all_blocks | ||
| end | ||
|
|
||
| def self.clear | ||
| @@all_blocks = [] | ||
| end | ||
|
|
||
| def self.find(block_id) | ||
| blocks = self.all | ||
|
|
||
| blocks.each do |block| | ||
| if block.block_id == block_id | ||
| return block | ||
| end | ||
| end | ||
|
|
||
| return nil | ||
| end | ||
|
|
||
| end # end of Block class | ||
|
|
||
| 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,79 @@ | ||
| require_relative 'room' | ||
| require_relative 'reservation' | ||
| require_relative 'block' | ||
|
|
||
| module Hotel | ||
|
|
||
| class Hotel | ||
| include Reservable | ||
|
|
||
| NUM_ROOMS = 20 | ||
|
|
||
| attr_reader :rooms | ||
|
|
||
| def initialize(num_rooms = NUM_ROOMS) | ||
| # check input | ||
| check_room_num(num_rooms) | ||
|
|
||
| @rooms = {} | ||
|
|
||
| # loop through num_rooms and add rooms to hash | ||
| (1..num_rooms).each do |num| | ||
| @rooms[num] = ::Hotel::Room.new(num) | ||
| end | ||
|
|
||
| end | ||
|
|
||
| def reserve(start_date, end_date, room_num) | ||
| check_dates(start_date, end_date) | ||
| room = @rooms[room_num] | ||
|
|
||
| raise ArgumentError.new("Room #{room_num} isn't available for the selected dates") if room.booked?(start_date, end_date) || room.blocked?(start_date, end_date) | ||
|
|
||
| return room.reserve(start_date, end_date) | ||
|
|
||
| end | ||
|
|
||
| def block(start_date, end_date, discount, room_nums) | ||
| block_rooms = room_nums.map { |room_num| rooms[room_num] } | ||
|
|
||
| raise ArgumentError.new("One or more rooms is unavailable for the selected dates") if block_rooms.any? { |room| room.booked?(start_date, end_date) || room.blocked?(start_date, end_date) } | ||
|
|
||
| return ::Hotel::Block.new(start_date, end_date, discount, block_rooms) | ||
| end | ||
|
|
||
| def reserve_blocked_room(block, room_num = nil) | ||
| # allows user to specify a room number; else system automatically | ||
| # choose a room from the block | ||
| avail_rooms = block.find_avail_in_block | ||
|
|
||
| raise ArgumentError.new("No more rooms available in block") if avail_rooms.length < 1 | ||
|
|
||
| # assign room num if not provided | ||
| room = room_num == nil ? avail_rooms.pop : rooms[room_num] | ||
|
|
||
| # if room num provided, check if available | ||
| if room_num && !block.room_available?(room) | ||
| raise ArgumentError.new("Room #{room_num} is already booked") | ||
| else | ||
| block.reserve(room) | ||
| end | ||
| end | ||
|
|
||
| def find_reservations_by_date(date) | ||
| return ::Hotel::Reservation.find_by_date(date) | ||
| end | ||
|
|
||
| def find_avail_rooms(start_date, end_date) | ||
| # returns a hash of rooms available in the date range | ||
|
|
||
| # check input | ||
| check_dates(start_date, end_date) | ||
|
|
||
| return rooms.reject { |room_num, room| room.booked?(start_date, end_date) || room.blocked?(start_date, end_date) } | ||
|
|
||
| end | ||
|
|
||
| end # end of Hotel class | ||
|
|
||
| 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,47 @@ | ||
| require 'date' | ||
| require_relative 'room' | ||
|
|
||
| module Reservable | ||
|
|
||
| def check_dates(start_date, end_date) | ||
|
|
||
| raise TypeError.new("#{start_date} must be of type Date") if start_date.class != Date | ||
| raise TypeError.new("#{end_date} must be of type Date") if end_date.class != Date | ||
| raise ArgumentError.new("Check out must be later than check in") if start_date >= end_date | ||
| raise ArgumentError.new("Can't reserve a room for a date before today") if start_date < Date.today | ||
|
|
||
| end | ||
|
|
||
| def check_room_num(num) | ||
| raise TypeError.new("#{num} must of type Integer") if num.class != Integer | ||
| raise ArgumentError.new("Invalid number of rooms") if num < 1 | ||
| end | ||
|
|
||
| def check_room(room) | ||
| raise TypeError.new("#{room} must be of type Hotel::Room") if room.class != Hotel::Room | ||
| end | ||
|
|
||
| def check_room_block(room_block, check_in, check_out) | ||
| raise TypeError.new("Block of rooms must be an array of Room objects") if room_block.class != Array || room_block[0].class != Hotel::Room | ||
|
|
||
| raise ArgumentError.new("Invalid number of rooms in block") if room_block.length > Hotel::Block::MAX_ROOMS || room_block.length < 1 | ||
|
|
||
| raise ArgumentError.new("Can't have unavailable rooms in the block") if room_block.any? { |room| room.booked?(check_in, check_out) || room.blocked?(check_in, check_out) } | ||
| end | ||
|
|
||
| def check_rate(rate) | ||
| raise TypeError.new("#{rate} must be of type Integer") if rate.class != Integer | ||
| raise ArgumentError.new("Rate must be greater than 0") if rate < 1 | ||
| end | ||
|
|
||
| def check_discount(discount) | ||
| # must be decimal representing percentage of full cost (e.g. 0.8 for 80% of orig rate) | ||
| raise TypeError.new("#{discount} must be of type Float") if discount.class != Float | ||
| raise ArgumentError.new("Not a discounted rate") if discount >= 1 || discount <= 0 | ||
| end | ||
|
|
||
| def include?(date) | ||
| return date >= @check_in && date < @check_out | ||
| 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,83 @@ | ||
| require 'date' | ||
| require_relative 'room' | ||
| require_relative 'reservable' | ||
|
|
||
| module Hotel | ||
|
|
||
| class Reservation | ||
| include Comparable | ||
| include Reservable | ||
|
|
||
| @@all_reservations = [] | ||
|
|
||
| attr_accessor :check_in, :check_out, :room_num, :rate, :reservation_id | ||
|
|
||
| def initialize(check_in, check_out, room, rate = room.rate) | ||
|
|
||
| check_dates(check_in, check_out) | ||
| check_room(room) | ||
|
|
||
| @reservation_id = @@all_reservations.length + 1 | ||
| @check_in = check_in | ||
| @check_out = check_out | ||
| @room_num = room.room_num | ||
| @rate = rate | ||
|
|
||
| @@all_reservations << self | ||
| end | ||
|
|
||
| def ==(other_reservation) | ||
| return check_in == other_reservation.check_in && check_out == other_reservation.check_out && room_num == other_reservation.room_num | ||
| end | ||
|
|
||
| # def include?(date) | ||
| # return date >= check_in && date < check_out | ||
| # end | ||
|
|
||
| def total_cost | ||
| num_nights = (check_out - check_in).to_i | ||
| return num_nights * rate | ||
| end | ||
|
|
||
| def self.all | ||
| return @@all_reservations | ||
| end | ||
|
|
||
| def self.find(reservation_id) | ||
| reservations = self.all | ||
|
|
||
| reservations.each do |reservation| | ||
| if reservation.reservation_id == reservation_id | ||
| return reservation | ||
| end | ||
| end | ||
|
|
||
| return nil | ||
| end | ||
|
|
||
| def self.clear | ||
| # clears all reservations (for testing) | ||
| @@all_reservations = [] | ||
| end | ||
|
|
||
| def self.find_by_date(date) | ||
| raise TypeError.new("#{date} must be of type Date") if date.class != Date | ||
|
|
||
| return @@all_reservations.select { |reservation| reservation.include?(date)} | ||
|
|
||
| end | ||
|
|
||
| def to_s | ||
| # return human readable representation | ||
| s = "Reservation id: #{reservation_id}\n" | ||
| s += "Room number: #{room_num}\n" | ||
| s += "Check-in: #{check_in}\n" | ||
| s += "Check-out: #{check_out}\n" | ||
| s += "Total cost: #{total_cost}\n" | ||
|
|
||
| return s | ||
|
|
||
| end | ||
|
|
||
| end # end of Reservation class | ||
| 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 following of the existing pattern, it makes it easier to add a CSV or database later.