-
Notifications
You must be signed in to change notification settings - Fork 45
Amy Cash - Pipes - Hotel #33
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?
Changes from all commits
9f72ccb
4f14110
56d1956
9cf882f
d426ac5
e43b6a8
1119537
fbe84f4
26a07a0
7615f62
8f200fc
4f0683c
d88b112
f0131f8
a8a7872
761e924
a36755e
4f78684
bb033c9
f7280d9
a0c397e
cd61e9b
2c55c4b
1a6dec9
6153a8d
afe2930
1e8092a
d983c4d
260ce6d
ead8db8
c28fbd2
a0f4148
7a137d6
14ca04e
a31c7ff
0debf89
25e1a7f
eee3121
3ecc2a6
387264d
830c402
b53c7a1
4033ec6
740d317
1466ac7
780118b
fbe32d5
9357171
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
|
|
||
| What classes does each implementation include? Are the lists the same? | ||
|
|
||
| Both implementations create the same Classes: CartEntry, ShoppingCart, and Order, but they are not the same in terms of how they work. | ||
|
|
||
| Write down a sentence to describe each class. | ||
|
|
||
| For implementation A: | ||
| CartEntry creates a CartEntry Object with unit_price and quantity instance variables that can be read and written to. | ||
|
|
||
| ShoppingCart creates a ShoppingCart Object with an entries array. | ||
|
|
||
| Order creates an Order object that contains an instance of the ShoppingCart class, which is assigned to an cart instance variable, as well as establishing a sales tax constant and a total_price method. | ||
|
|
||
| For implementation B: | ||
|
|
||
| CartEntry creates a CartEntry Object with unit_price and quantity instance variables, and a method that returns a total price based on those variables. | ||
|
|
||
| ShoppingCart create a ShoppingCart Object with an entries array, as well as a method for determining the total pre-tax price of the entries in the cart. | ||
|
|
||
| Order creates an Order object that contains an instance of the ShoppingCart class, which is assigned to a cart instance variable, as well as establishing a sales tax constant and a total_price method. | ||
|
|
||
| How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper. | ||
|
|
||
| -- For both implementations, I am assuming that there is a method somewhere else that allows you to add CartEntries to a ShoppingCart. | ||
|
|
||
| Implementation A: | ||
|
|
||
| Each object of the Order Class contains an instance of the ShoppingCart class, which contains CartEntries, which each have a price and quantity attached. The Order Class uses the unit_price, quantity, and SalesTax constant to determine a total order price. | ||
|
|
||
| Implementation B: | ||
|
|
||
| Each object of the Order Class contains an instance of the ShoppingCart class, which contains CartEntries. The price of each CartEntry is determined by the CartEntry class, using the unit_price and quantity for each entry. These prices are summed by the ShoppingCart class, to get a total price for the cart. The Order class then applies the appropriate tax to each order based on the ShoppingCart Price and the Tax Constant. | ||
|
|
||
|
|
||
| What data does each class store? How (if at all) does this differ between the two implementations? | ||
| What methods does each class have? How (if at all) does this differ between the two implementations? | ||
|
|
||
| Implementation A: | ||
|
|
||
| CartEntry stores unit_price and quantity, and has accessor methods for each of these. | ||
| ShoppingCart stores an array of entries, and has an accessor method for that array. | ||
| Order stores a SalesTax Constant and instances of the ShoppingCart class. It has a method for determining a total price including tax. | ||
|
|
||
| Implementation B: | ||
|
|
||
| CartEntry stores unit_price and quantity, but, unlike Implementation A, there is no accessor method for either variable. There is a price method that uses the variables to determine a price, and allow other methods to read this amount. | ||
|
|
||
| ShoppingCart stores an array of entries, but, unlike Implementation A, there is no accessor method for that array. There is a method that uses the CartEntry prices, as determined by the CartEntry class's price method, to determine a total price for the cart, which can be accessed by the Order Class. | ||
|
|
||
| Order stores a SalesTax Constant and instances of the ShoppingCart class. It has a method for determining a total price by applying sales tax to the cart price determined by the ShoppingCart Class's price method. | ||
|
|
||
| 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? | ||
|
|
||
| For Implementation A, the logic is contained in the Order Class. For Implementation B, it is spread out among all three classes. | ||
|
|
||
| Does total_price directly manipulate the instance variables of other classes? | ||
|
|
||
| For Implementation A, the Order Class manipulates the CartEntry Instance Variables, as well as the ShoppingCart entries. For Implementation B, the Classes don't manipulate Instance Variables of other classes, but make use of calculations made in those classes. | ||
|
|
||
| If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify? | ||
|
|
||
| This would be easier in Implementation B, because you could add a method to the CartEntry that would apply the necessary discount, which would then be passed along as part of the price. Whereas in Implementation A, you would have to try to put this into the Order Class. | ||
|
|
||
| Which implementation better adheres to the single responsibility principle? | ||
|
|
||
| Implementation B. | ||
|
|
||
| Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled? | ||
|
|
||
| Implementation B | ||
|
|
||
|
|
||
| Describe in design-activity.md what changes you would need to make to improve this design, and how why the resulting design would be an improvement. | ||
|
|
||
| The find_reservations method in the Hotel class uses the nights_reserved instance variable from the reservation class. As suggested in the feedback I got, I took this out and created a separate "includes_date" method in the Reservation class. If I want to change how the Nights Reserved works, it won't effect the Hotel Class as it would have before. I also look "length of stay" out of the reservation class, as it wasn't doing anything that the "nights_reserved" variable couldn't cover. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| require 'pry' | ||
| require_relative 'reservation' | ||
|
|
||
| module Hotel | ||
|
|
||
| class Block | ||
|
|
||
| attr_reader :block_list, :block_name | ||
|
|
||
| def initialize | ||
| @block_list = [] | ||
| @block_name = block_name | ||
| end | ||
|
|
||
| def add(id, day_in, day_out, discount, block_name, room) | ||
| @block_list << Reservation.new(id, day_in, day_out, discount, block_name, room) | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| require 'pry' | ||
| require 'date' | ||
|
|
||
| module Hotel | ||
|
|
||
| class DateRange | ||
|
|
||
| attr_reader :check_in, :check_out, :date_range | ||
|
|
||
| def initialize | ||
| end | ||
|
|
||
| def self.check_dates(first, last) | ||
| if Date.parse(first) > Date.parse(last) | ||
| raise ArgumentError, "Check Out Date is earlier than Check in Date" | ||
|
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. Is it valid if |
||
| end | ||
| end | ||
|
|
||
| def self.check_in(date) | ||
| @check_in = Date.parse(date) | ||
| return @check_in | ||
| end | ||
|
|
||
| def self.check_out(date) | ||
| @check_out = Date.parse(date) | ||
| return @check_out | ||
| end | ||
|
|
||
| def self.create_range(first, last) | ||
| date_range = [] | ||
| first.upto(last).each do |date| | ||
| date_range << date | ||
| end | ||
| return date_range | ||
| end | ||
|
|
||
| end #date class | ||
| end #module | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| require 'pry' | ||
| require 'date' | ||
| require_relative 'reservation' | ||
| require_relative 'reservation_list' | ||
| require_relative 'date_range' | ||
| require_relative 'block' | ||
|
|
||
| module Hotel | ||
| class Hotel | ||
|
|
||
| attr_reader :all_rooms, :reservations, :block | ||
|
|
||
| def initialize | ||
| @block = Block.new | ||
| @reservations = ReservationList.new | ||
| @all_rooms = {1 => 200, 2 => 200, 3 => 200, 4 => 200, 5 => 200, 6 => 200, 7 => 200, 8 => 200, 9 => 200, 10 => 200, 11 => 200, 12 => 200, 13 => 200, 14 => 200, 15 => 200, 16 => 200, 17 => 200, 18 => 200, 19 => 200, 20 => 200} | ||
| end | ||
|
|
||
| def make_reservation(id, day_in, day_out, discount: 0, block_name: nil, room: 0) | ||
| available = open_rooms(day_in, day_out) | ||
| if room == 0 | ||
|
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. Since room number is an integer, it might make more sense to use a non-integer sentinel value here. |
||
| first_open = available[0] | ||
| elsif room != 0 && available.include?(room) | ||
| first_open = room | ||
| else | ||
| raise ArgumentError, "Requested Room is Not Available during 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. I'm not sure an |
||
| end | ||
| @reservations.add(id, day_in, day_out, discount, block_name, room: first_open) | ||
| end | ||
|
|
||
| def find_reservations(day_in, day_out) | ||
| all_reservations = @reservations.reservation_list + @block.block_list | ||
| found_reservations = Array.new | ||
| DateRange.check_dates(day_in, day_out) | ||
| DateRange.create_range(day_in, day_out).each do |date| | ||
| found_reservations << all_reservations.find_all { |reservation| reservation.includes_date(date) } | ||
| end #each date | ||
| return found_reservations.flatten.uniq | ||
| end #find_reservations | ||
|
|
||
| def total_cost(reservation_id) | ||
| @reservations.reservation_list.each do |reservation| | ||
| if reservation.id == reservation_id | ||
| return reservation.total_cost | ||
| else | ||
| return "Reservation Not Found" | ||
|
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. This would be a good place to raise an error. Maybe a custom |
||
| end #end if | ||
| end #end each | ||
| end #end total cost | ||
|
|
||
| def open_rooms(day_in, day_out) | ||
| occupied_rooms = Array.new | ||
| find_reservations(day_in, day_out).each do |reservation| | ||
| occupied_rooms << reservation.room | ||
| end | ||
| open_rooms = (@all_rooms.keys - occupied_rooms) | ||
| return open_rooms | ||
| end | ||
|
|
||
| def create_block(id, day_in, day_out, discount: 0, number_of_rooms: 1, block_name: nil) | ||
| if number_of_rooms > 5 | ||
| return "A block cannot have more than 5 rooms" | ||
| end | ||
| available = open_rooms(day_in, day_out) | ||
| if available.length >= number_of_rooms | ||
| available.take(number_of_rooms).each do |room_number| | ||
| @block.add(id, day_in, day_out, discount, block_name, room: room_number) | ||
| end | ||
| else | ||
| raise ArgumentError, "Not Enough Rooms Available to Form Block" | ||
| end | ||
| end | ||
|
|
||
| def block_open_rooms?(block_name) | ||
| return true if @block.block_list.find { |reservation| block_name.include?(block_name) } | ||
| end | ||
|
|
||
| def reserve_in_block(block_name) | ||
| room = @block.block_list.find { |reservation| block_name.include?(block_name) } | ||
| @reservations.reservation_list << room | ||
| @block.block_list.delete(room) | ||
| end | ||
| end #class | ||
| end #module | ||
|
|
||
| #binding.pry | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| require 'pry' | ||
| require_relative 'date_range' | ||
|
|
||
| module Hotel | ||
| class Reservation | ||
|
|
||
| attr_reader :id, :check_in, :check_out, :room, :nights_reserved, :total_cost, :length_of_stay, :block_name | ||
|
|
||
| def initialize(id, day_in, day_out, discount, block_name, room: 0) | ||
| @id = id | ||
| @check_in = DateRange.check_in(day_in) | ||
| @check_out = DateRange.check_out(day_out) | ||
| @room = room | ||
| @nights_reserved = DateRange.create_range(@check_in, @check_out)[0..-2] | ||
| @discount = discount | ||
| @block_name = block_name | ||
| @total_cost = ((200 - discount) * @nights_reserved.length).to_i | ||
| end | ||
|
|
||
|
|
||
| def includes_date(date) | ||
| if @nights_reserved.include?(Date.parse(date)) | ||
| return true | ||
| end | ||
| end | ||
|
|
||
| def view_reservation | ||
| return "ID: #{@id}, Room: #{@room}, Check in: #{@check_in}, Check Out: #{@check_out}, Total Nights: #{@nights_reserved.length}, Total Cost: #{@total_cost}" | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| require 'pry' | ||
| require_relative 'reservation' | ||
|
|
||
| module Hotel | ||
|
|
||
| class ReservationList | ||
|
|
||
| attr_reader :reservation_list | ||
|
|
||
| def initialize | ||
| @reservation_list = [] | ||
| end | ||
|
|
||
| def add(id, day_in, day_out, discount, block_name, room) | ||
| @reservation_list << Reservation.new(id, day_in, day_out, discount, block_name, room) | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| #require 'pry' | ||
| require_relative 'spec_helper' | ||
|
|
||
| describe "DateRange Class" do | ||
| before do | ||
| @date_test = Hotel::DateRange.new | ||
| end #before | ||
|
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. You don't have any tests around |
||
|
|
||
| describe "Instantiation" do | ||
| before do | ||
| @test = Hotel::DateRange.new | ||
| end | ||
|
|
||
| it "can instantiate a DateRange class" do | ||
| Hotel::DateRange.new.must_be_kind_of Hotel::DateRange | ||
| end #instantiate DateRange | ||
| end #Instantiation | ||
|
|
||
| describe "Check In, Check Out, and Range Objects" do | ||
| it "can create a date object for a check in date: " do | ||
| Hotel::DateRange.check_in("2001/1/1").must_be_kind_of Date | ||
| end #check in | ||
|
|
||
| it "can create a date object for a check out date: " do | ||
| Hotel::DateRange.check_out("2001/1/10").must_be_kind_of Date | ||
| end #check_out | ||
|
|
||
| it "can create an array of dates between the check-in and check out" do | ||
| a = Hotel::DateRange.check_in("2001/1/1") | ||
| b = Hotel::DateRange.check_out("2001/1/10") | ||
| Hotel::DateRange.create_range(a, b).must_be_kind_of Array | ||
| end #date_range | ||
| end #check_in, check_out, range | ||
|
|
||
| end #DateRange | ||
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.
You never instantiate this class! You're basically using it as a library of methods to work with dates, which means it might be clearer as a module.