-
Notifications
You must be signed in to change notification settings - Fork 45
Pipes - Sarah Read-Brown - Hotel #47
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
627ed96
e7af91b
02af24f
78febe7
d608723
4deac25
17ec899
3b34daa
97fb45f
876b033
ccc6b10
95de54d
fd2fb92
d241790
6eb9948
3499b13
d495c37
08c533a
d86ef75
a7e7a94
d6a4885
3a12eb7
72797cc
ff52fe2
190c630
06b3aa8
46cf793
7fefb84
87141c0
0f12b18
0f7173e
a252fb1
4722c92
13dd2dc
1ebef79
bc65f72
0e03759
1380c48
097396b
570072f
2413e55
ad8791c
f4a76c9
e195727
a26eee4
92cb777
79a34c7
d36e017
27262e8
e3c9c59
d787e1f
dcd97fa
affc5f2
535c97b
7341b7f
da1e6f4
af98d6e
a5be186
e5f5e4c
bb57886
557305c
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 = true | ||
| t.test_files = FileList['specs/*_spec.rb'] | ||
| end | ||
|
|
||
| task default: :test |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| <!-- What classes does each implementation include? Are the lists the same? --> | ||
| Both implementations include the following three classes - CartEntry, ShoppingCart, and Order | ||
|
|
||
| <!-- Write down a sentence to describe each class. --> | ||
| The CartEntry class holds information on price and quantity for a given entry into a shopping cart. | ||
| ShoppingCart - is a collection of CartEntry objects. | ||
| Order - Responsible for the shopping cart and the total price of the cart. | ||
|
|
||
| <!-- 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 is the biggest class and draws upon the shopping cart class which in turn is in itself a collection of CartEntries. | ||
|
|
||
| <!-- What data does each class store? How (if at all) does this differ between the two implementations? --> | ||
| CartEntry: Stores the unit price and the quantity of each CartEntry object. | ||
| ShoppingCart: Stores a collection of CartEntry object in an array. | ||
| Order: Stores a ShoppingCart object. | ||
|
|
||
| <!-- What methods does each class have? How (if at all) does this differ between the two implementations? --> | ||
| CartEntry: | ||
| Implementation A - No methods. | ||
| Implementation B - A price method to calculate and return the price of the CartEntry. | ||
|
|
||
| ShoppingCart: | ||
| Implementation A - No methods. | ||
| Implementation B - Has a price method that looks at each entry in the shopping cart, calls the CartEntry price method, and updates the variable sum, then returns the sum variable. | ||
|
|
||
| Order: | ||
| Both implementations Order class have a total_price method. They each use the same structure to calculate and add sales tax but they ways they go about calculating the total price of the cart are they are quite different. | ||
|
|
||
| Implementation A - table the total_price method loops through each entry in the cart (CartEntry), calculates the price per entry, updates the sum variable with each entries price. Then the sales tax is calculates and added to the sum and the total_price is returned. | ||
|
|
||
| Implementation B - the Order class total_price method calls the price method built into the ShoppingCart class on the instance of the ShoppingCart object and stores the price into a variable. Then the sales tax is calculates and added to the sum and the total_price is returned. | ||
|
|
||
| Implementation A's total_price method is self contained while the total_price method in implementation B is dependent on the price method in each of the other classes. | ||
|
|
||
| <!-- 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? --> | ||
| In implementation A the total_price method logic is relatively self contained or retained while in Imp B the logic is delegated to both Shopping Cart and CartEntry classes by stringing their price methods together. | ||
|
|
||
| <!-- Does total_price directly manipulate the instance variables of other classes? --> | ||
| In implementation A the total_price directly manipulate the instance variables of the CartEntry instances while implementation B just calls the classes method on the instance of the class object. | ||
|
|
||
| <!-- If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify? --> | ||
| Implementation B would be easier to change because you could add some logic into the CartEntry class so that when a certain quantity is reached you discount the price. In order to do that in Implementation A you would have to add logic to the Orders class which is messy and would be out of place. | ||
|
|
||
| <!-- 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? --> | ||
|
|
||
|
|
||
| <!-- Based on the answers to the above questions, identify one place in your Hotel project where a class takes on multiple roles, or directly modifies the attributes of another class. 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. --> | ||
|
|
||
| I have sooo much going on in m reservations class. I felt that way when I was making the project and even more so after seeing the solution Dan posted. I started by following a suggestion of Dan's to move some of the responsibility of the check reservations method into the date range class. I honestly think there is more to do for me on this project to make it actually single responsibility but the CS Fun hw took up soo much of my time this past week that this hw didn't get the time and effort it deserved. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| require 'pry' | ||
| require 'date' | ||
| require_relative 'booking' | ||
|
|
||
| module Hotel | ||
| class Block < Booking | ||
| attr_reader :id, :rooms, :date_range, :nights, :total_cost, :block_info, :block_name, :discount, :reserved_rooms | ||
|
|
||
| def initialize(id, rooms, date_range, block_info) | ||
| super(id, rooms, date_range) | ||
| @block_info = block_info | ||
| end | ||
|
|
||
| def rooms_available_block | ||
| rooms_available = [] | ||
| @rooms.each do |room| | ||
| if room.booked == false || room.booked == nil | ||
| rooms_available << room | ||
| 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. This method for keeping track of which rooms are reserved is an interesting idea, but I believe you will run into trouble if you have multiple blocks for different dates containing the same rooms. Because the |
||
| end | ||
| return rooms_available | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| require 'pry' | ||
| require 'date' | ||
|
|
||
| module Hotel | ||
| class Booking | ||
| attr_reader :id, :rooms, :date_range, :nights, :total_cost, :block_info | ||
|
|
||
| def initialize(id, rooms, date_range, block_info: nil) #blockname and price? or whole block? | ||
| @id = id | ||
| @block_info = block_info | ||
| @rooms = rooms | ||
| @date_range = date_range | ||
| @nights = date_range.nights | ||
| @total_cost = (@nights * rooms.inject(0){|sum,room| sum + room.cost}) | ||
| end | ||
| # ran out of time to update total cost for a block, ideally I would pass through a discount and have that applied to total cost | ||
| # to book a roomin a block we would need to have total_cost update with the discounted range_of_dates | ||
| # def calc_total_cost | ||
| # if @block_info.length > 0 | ||
| # total = @rooms.length * @nights * @block_info[1] | ||
| # @total_cost = total | ||
| # else | ||
| # @total_cost = (@nights * rooms.inject(0){|sum,room| sum + room.cost}) | ||
| # end | ||
| # end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class InvalidDateRangeError < StandardError | ||
| end | ||
|
|
||
| class InvalidRoomQuantity < StandardError | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| require 'pry' | ||
| require 'date' | ||
|
|
||
| module Hotel | ||
| class DateRange | ||
| attr_reader :check_in, :check_out, :nights, :nights_arr | ||
|
|
||
| def initialize(check_in, check_out) | ||
| @check_in = check_in | ||
| @check_out = check_out | ||
| @nights = '' | ||
| @nights_arr = [] | ||
| make_nights_arr | ||
| end | ||
|
|
||
| def valid_input? | ||
| if @check_in.class != Date || @check_out.class != Date | ||
| raise ArgumentError.new("User input Check-in: #{@check_in} and Check-out: #{@check_out} are not valid inputs, must be a Date Object.") | ||
| end | ||
| end #end valid_input? method | ||
|
|
||
| def valid_date? | ||
| valid_input? | ||
| if @check_in > @check_out | ||
|
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. Nitpick: I like that these validations are called from your constructor, but I think the code would be a little more readable if they were invoked directly, rather than via |
||
| raise InvalidDateRangeError.new("Invalid Date Range: Check-out #{@check_out} is before Check-in #{@check_in}") | ||
| end | ||
| end #end valid_date? method | ||
|
|
||
| def make_nights_arr | ||
| valid_date? | ||
| @nights = (@check_out - @check_in).to_i | ||
| counter = 0 | ||
| @nights.times do | ||
| @nights_arr << (@check_in + counter) | ||
| counter += 1 | ||
| end | ||
| # return @nights_arr | ||
| end #end make_nights_arr method | ||
|
|
||
| def overlap?(other) | ||
| if other.check_out <= @check_in || other.check_in >= @check_out | ||
| return false | ||
| else | ||
| return true | ||
| end | ||
| end | ||
| end #end DateRange class | ||
| end #end Hotel module | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| require 'pry' | ||
| require 'date' | ||
|
|
||
| module Hotel | ||
| class Reservations | ||
| attr_reader :all_rooms, :all_reservations | ||
| def initialize | ||
| @all_rooms = [] | ||
| @all_reservations = [] | ||
| 20.times do |i| | ||
| i += 1 | ||
| @all_rooms << Hotel::Room.new(i) | ||
| end | ||
| end | ||
|
|
||
| def make_reservation(check_in,check_out,num_rooms, block_name:false) | ||
| date_range = DateRange.new(check_in,check_out) | ||
| id = (@all_reservations.length + 1) | ||
| rooms_available = check_availability(check_in,check_out) | ||
| enough_rooms?(rooms_available, num_rooms, block_name) | ||
| rooms_to_book = rooms_available.shift(num_rooms) | ||
|
|
||
| if block_name == false | ||
| update_room_state(rooms_to_book,num_rooms,true) | ||
| booking = Booking.new(id,rooms_to_book,date_range, block_info: false) | ||
| else | ||
| update_room_state(rooms_to_book,num_rooms,false) | ||
| booking = Block.new(id,rooms_to_book,date_range,block_name) | ||
| end | ||
| @all_reservations << booking | ||
| return booking | ||
| end | ||
|
|
||
| # booked_rooms = [] | ||
|
|
||
|
|
||
| def check_reservations(check_in,check_out) | ||
| booked_rooms = [] | ||
| range = DateRange.new(check_in,check_out) | ||
| @all_reservations.each do |booking| | ||
| if booking.date_range.overlap?(range) | ||
| booked_rooms << booking.rooms | ||
| end | ||
| end | ||
| return booked_rooms | ||
| # range = DateRange.new(check_in,check_out).nights_arr | ||
| # range.each do |date| | ||
| # @all_reservations.each do |booking| | ||
| # if booking.date_range.nights_arr.include?(date) | ||
| # booking.rooms.each do |room| | ||
| # if (!booked_rooms.include?(room)) | ||
| # booked_rooms << room | ||
| # end | ||
| # end | ||
| # end | ||
| # end | ||
| # end | ||
| return booked_rooms | ||
| end | ||
|
|
||
| def check_availability(check_in,check_out) | ||
| booked_rooms = check_reservations(check_in,check_out) | ||
| available_rooms = [] | ||
| all_rooms.each do |room| | ||
| if !(booked_rooms.include?(room)) | ||
| available_rooms << room | ||
| end | ||
| end | ||
| return available_rooms | ||
| end | ||
|
|
||
| def enough_rooms?(rooms_available, num_rooms, block_name) | ||
| if block_name != false && num_rooms > 5 | ||
| raise InvalidRoomQuantity.new("You cannot book more than 5 rooms in a block.") | ||
| end | ||
| if rooms_available.length < num_rooms || num_rooms > 20 | ||
| raise InvalidRoomQuantity.new("Unfortunately the hotel does not have enough available rooms to handle your request of #{num_rooms} rooms.") | ||
| end | ||
| end | ||
|
|
||
| def reserve_from_block(block, num_rooms) | ||
| rooms_to_book = block.rooms_available_block | ||
|
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 really like that you call a method on |
||
| enough_rooms?(rooms_to_book,num_rooms,false) | ||
| rooms_to_book = update_room_state(rooms_to_book,num_rooms,false) | ||
| id = all_reservations.length + 1 | ||
| booking = Booking.new(id,rooms_to_book,block.date_range, block_info:block.block_name) | ||
| return booking | ||
| end | ||
|
|
||
| def update_room_state(rooms_to_book,num_rooms,state) | ||
| num_rooms.times do |i| | ||
| rooms_to_book[i].booked = state | ||
| 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. You could use an |
||
| return rooms_to_book | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| require 'pry' | ||
| require 'date' | ||
|
|
||
| module Hotel | ||
| class Room | ||
| attr_reader :room_number, :cost, :booked | ||
| attr_accessor :booked | ||
|
|
||
| def initialize(room_number, booked: nil) | ||
| @room_number = room_number | ||
| @cost = 200 | ||
| @booked = booked | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| require_relative 'spec_helper' | ||
|
|
||
| describe "Block Class" do | ||
| before do | ||
| id = 1 | ||
| rooms = [Hotel::Room.new(3),Hotel::Room.new(2),Hotel::Room.new(1)] | ||
| date_range = Hotel::DateRange.new(Date.new(2017,3,9),Date.new(2017,3,13)) | ||
| @wedding = Hotel::Block.new(id, rooms, date_range, "Royal Wedding") | ||
| end | ||
| it "can be instantiated" do | ||
| @wedding.must_be_kind_of Hotel::Block | ||
| end | ||
| it "must respond to all intance variables" do | ||
| @wedding.must_respond_to :id | ||
| @wedding.must_respond_to :total_cost | ||
| @wedding.must_respond_to :rooms | ||
| @wedding.must_respond_to :date_range | ||
| @wedding.must_respond_to :nights | ||
| @wedding.must_respond_to :block_info | ||
| @wedding.must_respond_to :reserved_rooms | ||
| end | ||
| describe "rooms_available_block" do | ||
| it "returns an array with the available rooms" do | ||
| @wedding.rooms_available_block.length.must_equal 3 | ||
| @wedding.rooms_available_block.must_be_kind_of Array | ||
| end | ||
| it "returns rooms objects" do | ||
| @wedding.rooms_available_block[0].must_be_kind_of Hotel::Room | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| require_relative 'spec_helper' | ||
|
|
||
| describe "Booking class" do | ||
| before do | ||
| id = 1 | ||
| rooms = [Hotel::Room.new(15),Hotel::Room.new(16)] | ||
| date_range = Hotel::DateRange.new(Date.new(2017,9,5),Date.new(2017,9,8)) | ||
| block_info = [] | ||
| @reservation = Hotel::Booking.new(id, rooms, date_range) | ||
| end | ||
| describe "Initialize" do | ||
| it "can be instantiated" do | ||
| @reservation.must_be_kind_of Hotel::Booking | ||
| end | ||
| it "must respond to all intance variables" do | ||
| @reservation.must_respond_to :id | ||
| @reservation.must_respond_to :total_cost | ||
| @reservation.must_respond_to :rooms | ||
| @reservation.must_respond_to :date_range | ||
| @reservation.must_respond_to :nights | ||
| @reservation.must_respond_to :block_info | ||
| end | ||
| it "total cost must return the appropriate amount for 2 rooms, and be an Integer" do | ||
| # binding.pry | ||
| @reservation.total_cost.must_equal 1200 | ||
| end | ||
| it "total cost must be an integer" do | ||
| @reservation.total_cost.must_be_kind_of Integer | ||
| end | ||
| end | ||
| end |
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.
Because this method is already inside the
Blockclass, you shouldn't need to add_blockto the name.