-
Notifications
You must be signed in to change notification settings - Fork 46
Melissa O'Hearn : Hotel : Edges #34
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
4200d5b
0d29cf2
b1cd92f
66869e9
7038e06
f0e9993
51d91ea
76947f0
323aca3
94cee4f
23fe9f2
61e5f8d
6ff8085
f8ec590
4334e89
1045d1c
e8b1e84
8a81961
e1c7963
abaacfc
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,130 @@ | ||
| Implementation A | ||
| class CartEntry | ||
| attr_accessor :unit_price, :quantity | ||
| def initialize(unit_price, quantity) | ||
| @unit_price = unit_price | ||
| @quantity = quantity | ||
| end | ||
| end | ||
|
|
||
| class ShoppingCart | ||
| attr_accessor :entries | ||
| def initialize | ||
| @entries = [] | ||
| end | ||
| end | ||
|
|
||
| class Order | ||
| SALES_TAX = 0.07 | ||
| def initialize | ||
| @cart = ShoppingCart.new | ||
| end | ||
|
|
||
| def total_price | ||
| sum = 0 | ||
| @cart.entries.each do |entry| | ||
| sum += entry.unit_price * entry.quantity | ||
| end | ||
| return sum + sum * SALES_TAX | ||
| end | ||
| end | ||
| Implementation B | ||
| class CartEntry | ||
| def initialize(unit_price, quantity) | ||
| @unit_price = unit_price | ||
| @quantity = quantity | ||
| end | ||
|
|
||
| def price | ||
| return @unit_price * @quantity | ||
| end | ||
| end | ||
|
|
||
| class ShoppingCart | ||
| def initialize | ||
| @entries = [] | ||
| end | ||
|
|
||
| def price | ||
| sum = 0 | ||
| @entries.each do |entry| | ||
| sum += entry.price | ||
| end | ||
| return sum | ||
| end | ||
| end | ||
|
|
||
| class Order | ||
| SALES_TAX = 0.07 | ||
| def initialize | ||
| @cart = ShoppingCart.new | ||
| end | ||
|
|
||
| def total_price | ||
| subtotal = @cart.price | ||
| return subtotal + subtotal * SALES_TAX | ||
| end | ||
| end | ||
|
|
||
|
|
||
| What classes does each implementation include? Are the lists the same? | ||
| Implementation A classes: | ||
| 1 - CartEntry | ||
| 2 - ShoppingCart | ||
| 3 - Order | ||
|
|
||
| Implementation B classes: | ||
| 1 - CartEntry | ||
| 2 - ShoppingCart | ||
| 3 - Order | ||
|
|
||
| The lists are the same | ||
|
|
||
| Write down a sentence to describe each class. | ||
| Implementation A classes: | ||
| 1 - CartEntry: Initializes a new instance of an item's price and quantity | ||
| 2 - ShoppingCart: Stores all the entries in an array | ||
| 3 - Order: Creates a new instance of a ShoppingCart, and calculates the total price of items in the cart | ||
|
|
||
| Implementation B classes: | ||
| 1 - CartEntry: Same as A | ||
| 2 - ShoppingCart: Same as A | ||
| 3 - Order: Same as A | ||
|
|
||
| How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper. | ||
| In A, the CartEntry class instantiates 2 variables, @unit_price and @quantity. These are used in the the Order class to calculate sum. IN the class ShoppingCart, @entries is instantiated. this instance variable is also used in the Order class to also calculate the sum | ||
|
|
||
| In B, CartEntry class instantiates @unit_price and @quantity which are used in a method within the class. This method uses these variables to return a price. In ShoppingCart class, @entries in instantiated and another price method is created, this method uses the price from the CartEntry class to calculate the sum of items in the cart. This price is then used in the Order class to calculate the total price of all items | ||
|
|
||
|
|
||
| What data does each class store? How (if at all) does this differ between the two implementations? | ||
| In A & B, CartEntry stores unit_price and quantity. | ||
| ShoppingCart stores entries | ||
| Order stores SALES_TAX, cart | ||
|
|
||
|
|
||
| What methods does each class have? How (if at all) does this differ between the two implementations? | ||
| IN A, the only class that has methods is Order. This calculates total_price. | ||
|
|
||
| In B, CartEntry stores price method. | ||
| ShoppingCart stores another price method. | ||
| Order stores a total_price method like A | ||
|
|
||
| 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 A, it is retained in Order | ||
| In B, it is delegated to lower level classes | ||
|
|
||
| Does total_price directly manipulate the instance variables of other classes? | ||
| In A, total_price does manipulate the instance variables of other classes | ||
| In B, total_price does not manipulate them | ||
|
|
||
| If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify? | ||
| In B, we could create another class that would have bulk prices that could return a bulk price that could be added to the order class. Or just add a bulk_price to CartEntry that would be added to the price method. | ||
| In A, a bulk_price could be added to CartEntry but then Order would also have to be changed to add it there too. | ||
|
|
||
| Which implementation better adheres to the single responsibility principle? | ||
| B | ||
|
|
||
| Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled? | ||
| B |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| require 'date' | ||
| require_relative "reservation" | ||
| require_relative "date_range" | ||
| require 'pry' | ||
|
|
||
| module Hotel | ||
| class Block | ||
|
|
||
| # As an administrator, I can create a block of rooms | ||
| # To create a block you need a date range, collection of rooms and a discounted room rate | ||
| # The collection of rooms should only include rooms that are available for the given date range | ||
| # If a room is set aside in a block, it is not available for reservation by the general public, nor can it be included in another block | ||
|
|
||
| attr_reader :collection_of_rooms, :date_range, :cost_per_night | ||
|
|
||
| def initialize(collection_of_rooms, cost_per_night, check_in, check_out) | ||
|
|
||
| @collection_of_rooms = collection_of_rooms | ||
| @cost_per_night = cost_per_night | ||
| @date_range = Hotel::DateRange.new(check_in, check_out) | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| require 'date' | ||
| require_relative "reservation" | ||
| require_relative "date_range" | ||
| require_relative "block" | ||
| require 'pry' | ||
|
|
||
|
|
||
| module Hotel | ||
| class BookingSystem | ||
| attr_reader :rooms, :reservations | ||
|
|
||
| def initialize | ||
| @rooms = | ||
| [ | ||
| {room_number: 1}, {room_number: 2}, {room_number: 3}, {room_number: 4}, {room_number: 5}, {room_number: 6}, {room_number: 7}, {room_number: 8}, {room_number: 9}, {room_number: 10}, {room_number: 11}, {room_number: 12}, {room_number: 13}, {room_number: 14}, {room_number: 15}, {room_number: 16}, {room_number: 17}, {room_number: 18}, {room_number: 19}, {room_number: 20} | ||
| ] | ||
|
|
||
| @reservations = [ ] | ||
|
|
||
| end | ||
|
|
||
| # method to make_reservation | ||
| # return if successful return res_id | ||
| def make_reservation(cost_per_night, check_in, check_out) | ||
| available_rooms = list_available_rooms(Hotel::DateRange.new(check_in, check_out)) | ||
| if available_rooms.empty? | ||
| raise StandardError, "There's no room in the inn" | ||
|
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 create a def make_reservation(cost_per_night, check_in, check_out)
date_range = Hotel::DateRange.new(check_in, check_out)
available_rooms = list_available_rooms(date_range)
# ...
reservation = Hotel::Reservation.new(room_number, cost_per_night, date_range)Of course, you would have to adjust your constructor for |
||
| end | ||
| room_number = available_rooms.sample[:room_number] | ||
| reservation = Hotel::Reservation.new(room_number, cost_per_night, check_in, check_out) | ||
| @reservations << reservation | ||
| return reservation | ||
| end | ||
|
|
||
| #make block reservation | ||
| # error if not between 3-5 rooms | ||
| def make_block_reservation(number_of_rooms, cost_per_night, check_in, check_out) | ||
| available_rooms = list_available_rooms(Hotel::DateRange.new(check_in, check_out)) | ||
| if available_rooms.length < number_of_rooms | ||
| raise StandardError, "There's no room in the inn" | ||
| end | ||
| block_reservation = [] | ||
| number_of_rooms.times do | ||
| room_number = available_rooms.sample[:room_number] | ||
| reservation = Hotel::Reservation.new(room_number, cost_per_night, check_in, check_out) | ||
| @reservations << reservation | ||
| block_reservation << res | ||
| end | ||
| return block_reservation | ||
| end | ||
|
|
||
| # list reservations for a specific date | ||
| def reservations_by_date(date) | ||
| date = Date.parse(date) | ||
| res_by_date = @reservations.select do |res| | ||
| res.date_range.included_in_date_range(date) | ||
| end | ||
| return res_by_date | ||
| end | ||
|
|
||
| # list reservations for a specific date range | ||
| def reservations_by_date_range(date_range) | ||
| res_by_date_range = @reservations.select do |res| | ||
| res.date_range.overlaps?(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 love that you broke this out as a separate method - it makes 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 is also a great example of loosely coupled code - instead of having to do math on the individual dates stored in the reservation, this code is able to call another method ( |
||
| end | ||
| return res_by_date_range | ||
| end | ||
|
|
||
| # As an administrator, I can view a list of rooms that are not reserved for a given date range | ||
| # reservation date range overlaps with | ||
| def list_available_rooms(date_range) | ||
| conflicting_reservations = reservations_by_date_range(date_range) | ||
|
|
||
| available_rooms = @rooms.reject do |room| | ||
| conflicting_reservations.find do |res| | ||
| res.room_number == room[:room_number] | ||
| end | ||
| end | ||
| return available_rooms | ||
| end | ||
|
|
||
|
|
||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| require 'date' | ||
| require_relative '../lib/reservation' | ||
|
|
||
| module Hotel | ||
| class DateRange | ||
|
|
||
| attr_reader :check_in, :check_out | ||
|
|
||
| def initialize(check_in, check_out) | ||
|
|
||
| @check_in = Date.parse(check_in) | ||
| @check_out = Date.parse(check_out) | ||
|
|
||
| unless @check_out > @check_in | ||
| raise StandardError.new("Check-out can not be before check-in. Chech-out is: #{@check_out} check-in is #{@check_in}") | ||
| 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. Instead of raising a |
||
|
|
||
| end | ||
| # -Find duration of stay | ||
| def duration_of_stay | ||
| duration_of_stay = (@check_out - @check_in).to_i | ||
| return duration_of_stay | ||
| end | ||
|
|
||
| # include? method | ||
| def included_in_date_range(date) | ||
| return date.between?(check_in, check_out) | ||
| end | ||
|
|
||
| # overlap? method | ||
|
|
||
| def overlaps?(date_range) | ||
| overlap = @check_in < date_range.check_out && date_range.check_in < @check_out | ||
| return overlap | ||
| 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. Good work getting this tricky bit of logic figured out. |
||
|
|
||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # require_relative '../lib/date_range' | ||
| require 'pry' | ||
|
|
||
|
|
||
| module Hotel | ||
| class Reservation | ||
|
|
||
| attr_reader :room_number, :cost_per_night, :date_range, :reservation_id | ||
|
|
||
| def initialize(room_number, cost_per_night, check_in, check_out) | ||
| @room_number = room_number | ||
| @cost_per_night = cost_per_night.to_f | ||
| @date_range = Hotel::DateRange.new(check_in, check_out) | ||
| @reservation_id = Reservation.generate_id | ||
| end | ||
|
|
||
| # - Generate an reservation ID | ||
| def self.generate_id | ||
| rand_array = [] | ||
|
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 is rather a lot of work, and more importantly it's not guaranteed to give you a unique ID. Another way to approach this would be to assign numeric IDs sequentially, starting at 1 and counting up - this is what Rails does. Something like this: @@next_id = 1
def assign_id
id = @@next_id
@@next_id += 1
return id
end |
||
|
|
||
| 3.times do | ||
| rand_num = rand(1..9).to_s | ||
| rand_array << rand_num | ||
| end | ||
|
|
||
| 3.times do | ||
| rand_letter = ('A'..'Z').to_a.sample.to_s | ||
| rand_array << rand_letter | ||
| end | ||
|
|
||
| rand_array.insert(3, "-") | ||
| return rand_array * "".to_s | ||
| end | ||
| # - Find total cost | ||
| def total_cost | ||
| total_cost = @cost_per_night * @date_range.duration_of_stay | ||
| return total_cost | ||
| end | ||
|
|
||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| - Finish the blocks class. | ||
| -- The collection of rooms should only include rooms that are available for the given date range | ||
| -- If a room is set aside in a block, it is not available for reservation by the general public, nor can it be included in another block | ||
|
|
||
| - Some names could be changed/condensed: | ||
| -- list_available_rooms | ||
| -- reservations_by_date_range | ||
| -- blocks.rb change to room_block? | ||
|
|
||
|
|
||
| - Tests could be better organized: | ||
| -- in date_range, most could fit under "date range" | ||
| -- Tests could have more edge chases - If no date is passed in, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| require 'minitest' | ||
| require 'minitest/autorun' | ||
| require 'minitest/reporters' | ||
|
|
||
| require_relative 'spec_helper' | ||
| require_relative '../lib/booking_system.rb' | ||
| require_relative '../lib/reservation.rb' | ||
| require_relative '../lib/date_range.rb' | ||
|
|
||
| describe "initialize" do | ||
| before do | ||
| @new_block_res = Hotel::Block.new([{room_number: 1}, {room_number: 2}, {room_number: 3}, {room_number: 4}], 200.00, "2018-02-03", "2018-02-06") | ||
| end | ||
|
|
||
| it "is an instance of Blocks" do | ||
| expect(@new_block_res).must_be_kind_of Hotel::Block | ||
| end | ||
|
|
||
| it "Takes date_range, collection_of_rooms and cost per night" do | ||
|
|
||
| expect(@new_block_res).must_respond_to :date_range | ||
| expect(@new_block_res).must_respond_to :collection_of_rooms | ||
| expect(@new_block_res).must_respond_to :cost_per_night | ||
|
|
||
| end | ||
|
|
||
| it "is set up for specific attributes and data types" do | ||
| [:date_range, :collection_of_rooms, :cost_per_night].each do |initial| | ||
| expect(@new_block_res).must_respond_to initial | ||
| end | ||
|
|
||
| expect(@new_block_res.date_range).must_be_kind_of Hotel::DateRange | ||
| expect(@new_block_res.collection_of_rooms).must_be_kind_of Array | ||
| expect(@new_block_res.cost_per_night).must_be_kind_of Float | ||
|
|
||
|
|
||
| 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.
This data structure is a little more complex than it needs to be - I would probably keep a list of integers rather than making them all hashes. This ends up making some of your code below more complex as well.