-
Notifications
You must be signed in to change notification settings - Fork 46
Hayden (Edges) - Hotel #42
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
aefb9f1
08989e9
fa4a9dd
f726616
1bba710
f7e7024
a0e8024
3cd6c04
9d99057
7c4892f
5bf6222
22f046d
9f1a580
9451604
257423d
cf3732a
7c5f7d5
eb47cdc
d3f6c12
e1cb082
d917169
5808a9a
d388c3e
dff0585
886aaab
833f7ff
b38fe09
aeb934c
52e41c2
b4087df
3288ef2
43e3e12
21cec25
7760b1a
dbed7c0
04cc141
6ef98f6
07f110a
6fe0089
afd0c78
deb9bf5
5faca3a
f427b94
627a1af
ce4c995
fd332ca
81c33ea
0dce526
85ee089
d57fd52
185789a
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,41 @@ | ||
| 1. Both implementations contains the same three classes--CartEntry, ShoppingCart, and Order. | ||
|
|
||
| 2. CartEntry - Responsible for a single entry in a ShoppingCart, tracks unit_price of an item and quantity of that item being purchased. | ||
|
|
||
| ShoppingCart - Responsible for multiple CartEntries. | ||
|
|
||
| Order - Responsible for calculating total price of a particular ShoppingCart | ||
|
|
||
| 3. CartEntry (one or many) | ||
| v | ||
| ShoppingCart (only one) | ||
| v | ||
| Order | ||
|
|
||
| 4. CartEntry - price of a single unit of an item, quantity of that item being purchased | ||
| ShoppingCart - holds multiple individual CartEntries | ||
| Order - sales tax constant, a single ShoppingCart | ||
|
|
||
| The data stored by each class does not change between implementations | ||
|
|
||
| 5. CartEntry - #price returns the price of a quantity of some item | ||
| ShoppingCart - #price returns the price of all entries in the cart | ||
| Order - #total_price returns the cart price + sales tax | ||
|
|
||
| Implementation A uses attr_accessor to make the data from CartEntry and ShoppingCart available to Order so it can calculate the total price, but Implementation B provides instance methods for each that return their respective prices, so that Order can simply call @cart.price then add sales tax. | ||
|
|
||
| 6. In Implementation B, the price logic is delegated to lower level classes, but in Implementation A it is not. | ||
|
|
||
| total_price does directly manipulate the instance variables of other classes in Implementation A, but not in B. | ||
|
|
||
| 7. Ideally we would be working with Implementation B and could just add conditional functionality in CartEntry#price that affected unit price based on quantity. In Implementation A we would need to modify the code in Order#total_price, which is both unintuitive and messy. | ||
|
|
||
| 8. Implementation B, absolutely. I've made my case above... lol | ||
|
|
||
| 9. Once again, B. | ||
|
|
||
|
|
||
| ----------- | ||
|
|
||
|
|
||
| In RoomBooker#new_block_reservation, the local `room` variable is used to modify the state of the Room struct, which belongs to the RoomBlock instance. If I were to handle this functionality instead within the RoomBlock class itself in an instance method, my classes would be even more loosely coupled and resilient to future change/easier to change in the future. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| module DateLogic | ||
| def DateLogic.date_range_include?(reservation, date) | ||
| if (reservation.check_in...reservation.check_out).cover?(date) | ||
| return true | ||
| else | ||
| return false | ||
| 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. Would it be as readable/work as well if we one-lined this to: return reservation.check_in...reservation.check_out).cover?(date) |
||
| end | ||
|
|
||
| def DateLogic.date_ranges_exclusive?( | ||
| old_check_in, | ||
| old_check_out, | ||
| new_check_in, | ||
| new_check_out) | ||
|
|
||
| existing_range_array = date_range_array(old_check_in, old_check_out) | ||
|
|
||
| new_range_array = date_range_array(new_check_in, new_check_out) | ||
|
|
||
| intersecting_dates = existing_range_array & new_range_array | ||
|
|
||
| if intersecting_dates.empty? | ||
| return true | ||
| else | ||
| return false | ||
| end | ||
| end | ||
|
|
||
| def DateLogic.date_range_array(check_in, check_out) | ||
| date_range = check_in...check_out | ||
| return date_range.to_a | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| class NoRoomsInBlock < StandardError | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| module BookingLogic | ||
| class Reservation | ||
| attr_reader :room, :check_in, :check_out | ||
|
|
||
| def initialize(room, check_in, check_out) | ||
| check_date_range(check_in, check_out) | ||
| @room = room | ||
| @check_in = check_in | ||
| @check_out = check_out | ||
| end | ||
|
|
||
| def check_date_range(check_in, check_out) | ||
| if check_out <= check_in | ||
| raise StandardError, "Invalid date range provided" | ||
| end | ||
| end | ||
|
|
||
| def reservation_cost | ||
| days_reserved = check_out - check_in | ||
| return days_reserved.to_i * room.cost | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| module BookingLogic | ||
| class Room | ||
| end | ||
| 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. Don't forget to remove unused files before you submit your project/as you refactor them out! |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| require_relative 'no_rooms_in_block' | ||
|
|
||
| module BookingLogic | ||
| class RoomBlock | ||
| attr_reader :name, :check_in, :check_out, :rooms, :rate | ||
|
|
||
| def initialize(name, check_in, check_out, room_block, rate) | ||
| check_number_of_rooms(room_block) | ||
| @name = name | ||
| @check_in = check_in | ||
| @check_out = check_out | ||
| @rooms = room_block | ||
| @rate = rate | ||
| end | ||
|
|
||
| def check_number_of_rooms(room_block) | ||
| if room_block.length > 5 | ||
| raise StandardError, "Maximum of 5 rooms per room block" | ||
| end | ||
| end | ||
|
|
||
| def available | ||
| available_rooms = rooms.find_all { |room| room.block_reserved == nil } | ||
|
|
||
| if available_rooms.empty? | ||
| raise NoRoomsInBlock, "No rooms currently available in this block" | ||
| else | ||
| return available_rooms | ||
| end | ||
| end | ||
|
|
||
| def set_blocked_room_rate | ||
| rooms.each do |room| | ||
| room.cost = rate | ||
| end | ||
| end | ||
|
|
||
| def reserve_room | ||
| room = self.available.first | ||
| room.block_reserved = true | ||
| return room | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,169 @@ | ||
| require 'date' | ||
| require_relative 'date_logic' | ||
| require_relative 'reservation' | ||
| require_relative 'room_block' | ||
| require_relative 'room_not_available' | ||
|
|
||
| module BookingLogic | ||
| class RoomBooker | ||
| Room = Struct.new(:id, :cost, :block_reserved) | ||
|
|
||
| attr_reader :rooms | ||
| attr_accessor :reservations, :blocks | ||
|
|
||
| def initialize | ||
| @rooms = populate_rooms | ||
| @reservations = [] | ||
| @blocks = [] | ||
| end | ||
|
|
||
| def populate_rooms | ||
| rooms_array = [] | ||
|
|
||
| 20.times do |id| | ||
| room = Room.new((id + 1), 200) | ||
| rooms_array << room | ||
| end | ||
|
|
||
| return rooms_array | ||
| end | ||
|
|
||
| def list_rooms | ||
| return rooms | ||
| end | ||
|
|
||
| def find_room_by_id(room_id) | ||
| return rooms.find { |room| room.id == room_id } | ||
| end | ||
|
|
||
| def list_reservations(date) | ||
| list_of_reservations = [] | ||
|
|
||
| reservations.each do |reservation| | ||
| if DateLogic.date_range_include?(reservation, date) | ||
| list_of_reservations << reservation | ||
| end | ||
| 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 might be a good opportunity to use |
||
|
|
||
| return list_of_reservations | ||
| end | ||
|
|
||
| def list_available_rooms(check_in, check_out) | ||
| reserved_rooms = [] | ||
|
|
||
| reservations.each do |reservation| | ||
| unless DateLogic.date_ranges_exclusive?( | ||
| reservation.check_in, | ||
| reservation.check_out, | ||
| check_in, | ||
| check_out | ||
| ) | ||
|
|
||
| reserved_rooms << reservation.room | ||
| end | ||
| end | ||
|
|
||
| unavailable_rooms = reserved_rooms + find_blocked_rooms(check_in, check_out) | ||
|
|
||
| available_rooms = rooms.dup | ||
|
|
||
| unavailable_rooms.each do |unavailable_room| | ||
| available_rooms.delete_if { |room| room.id == unavailable_room.id } | ||
| 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 block of code is wonderful ✨ I love it! I kept looking at this block of code and thinking of a way to improve it. I thought of a couple ways that could potentially be less readable, like subtracting the two arrays (which I'm unsure works with structs), or using a destructive |
||
|
|
||
| return available_rooms | ||
| end | ||
|
|
||
| def new_reservation(room_id, check_in, check_out) | ||
| room = find_room_by_id(room_id) | ||
| room_unavailable?(room, check_in, check_out) | ||
| new_reservation = BookingLogic::Reservation.new(room, check_in, check_out) | ||
| reservations << new_reservation | ||
| return new_reservation | ||
|
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 just want to call out that we never formally teach this pattern, but this is a great and common pattern: when a method makes something new, it returns that new thing in the end. Nice! |
||
| end | ||
|
|
||
| def room_unavailable?(room, check_in, check_out) | ||
| unless list_available_rooms(check_in, check_out).include?(room) | ||
| raise RoomNotAvailable, "Room not available for the given dates" | ||
| end | ||
| end | ||
|
|
||
| def find_block(name) | ||
| found_block = blocks.find { |block| block.name == name } | ||
| return found_block | ||
| end | ||
|
|
||
| def find_blocked_rooms(check_in, check_out) | ||
| blocked_rooms = [] | ||
|
|
||
| blocks.each do |block| | ||
| unless DateLogic::date_ranges_exclusive?( | ||
| block.check_in, | ||
| block.check_out, | ||
| check_in, | ||
| check_out | ||
| ) | ||
|
|
||
| block.rooms.each do |room| | ||
| blocked_rooms << room | ||
| end | ||
| end | ||
| end | ||
|
|
||
| return blocked_rooms | ||
| end | ||
|
|
||
| def block_available_rooms(check_in, check_out, number_of_rooms, rate) | ||
|
|
||
| available_rooms = list_available_rooms(check_in, check_out) | ||
|
|
||
| room_block = [] | ||
|
|
||
| number_of_rooms.times do |i| | ||
| room_block << available_rooms[i].dup | ||
| end | ||
|
|
||
| if room_block.length < number_of_rooms | ||
| raise StandardError, "There are not enough available rooms for the given dates to create this room block" | ||
| end | ||
|
|
||
| return room_block | ||
| end | ||
|
|
||
| def new_room_block(name, check_in, check_out, number_of_rooms, rate) | ||
|
|
||
| block_of_rooms = block_available_rooms( | ||
| check_in, | ||
| check_out, | ||
| number_of_rooms, | ||
| rate | ||
| ) | ||
|
|
||
| new_room_block = BookingLogic::RoomBlock.new( | ||
| name, | ||
| check_in, | ||
| check_out, | ||
| block_of_rooms, | ||
| rate | ||
| ) | ||
|
|
||
| new_room_block.set_blocked_room_rate | ||
|
|
||
| @blocks << new_room_block | ||
| return new_room_block | ||
| end | ||
|
|
||
| def new_block_reservation(name) | ||
| block = find_block(name) | ||
| room = block.reserve_room | ||
| new_reservation = BookingLogic::Reservation.new(room, block.check_in, block.check_out) | ||
| reservations << new_reservation | ||
| return new_reservation | ||
| end | ||
|
|
||
| def set_room_rate(room_id, custom_rate) | ||
|
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 think this method is unused? I think you probably wrote this and then realized you should move it to the RoomBlock class, which you did... but you left this code here! You can probably get rid of this if that's the case |
||
| room = rooms.find { |room| room.id == room_id } | ||
| room.cost = custom_rate | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| class RoomNotAvailable < StandardError | ||
| 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 is one of the most appropriate ways to use a module and I'm very very very happy with this.