-
Notifications
You must be signed in to change notification settings - Fork 40
Space - Lee #28
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?
Space - Lee #28
Changes from all commits
f056858
3144873
19952f0
90bd045
a00f3af
306f359
0e64e25
97eb647
35e9edd
866a9d2
de5b3ae
394d755
418c78f
0b97375
bbe2fda
aaee32a
78c9850
20c3bb7
1df375f
0b9fee8
4aa4764
9b145a8
53a82f1
3b79d82
37d2fd8
31d256d
39b6c93
030b0d3
d3328f8
71f6fa0
ba4e102
7f3d5ff
2aab015
f796015
331bf77
abbe760
71fa3c9
9abeb0b
a5d29a1
86db0ec
b87155e
db6f02c
a3e4a2f
4d9eaf1
3966ea3
5866515
a35d800
8adbae0
03cd299
d00e780
26f8657
2a561de
dc157fa
676dbb3
ba9ee7e
aa4ccbf
554bfcc
623bc3e
e7b0903
c47e6e7
7e3acf0
b6930c2
4ca32d3
6564809
87c63d5
cd15bfa
de91b4e
38bc2ff
96cb9b3
2670238
bbc8a82
ba556d2
b00b3cc
fa831a1
86050da
08621c5
16cdb77
ff5ebed
28db7f9
5f3adcb
fc88917
2046f2e
4de7cee
f1023dc
e78198e
9780529
6a9ee84
6c13c76
62163c9
9b67631
b17de34
2f1f41c
1aa8991
634fc4b
0e3bff9
bd7e40d
edff9dd
f200014
3363369
c6689d5
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 |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| .DS_Store | ||
| *.gem | ||
| *.rbc | ||
| /.config | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,17 @@ | ||||||
| module Hotel | ||||||
| class 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. This is too much indentation. You should configure VS Code to indent by two spaces. |
||||||
| attr_reader :id, :rooms, :rate, :date_range | ||||||
|
|
||||||
| def initialize(id:, rooms:, rate:, start_date:, end_date:) | ||||||
| @id = id | ||||||
| @rooms = rooms | ||||||
| @rate = rate | ||||||
| @date_range = Hotel::DateRange.new( | ||||||
| start_date: start_date, | ||||||
| end_date: end_date | ||||||
| ) | ||||||
|
|
||||||
| raise ArgumentError.new("Block must contain between 2 and 5 rooms!") if @rooms.length < 2 || @rooms.length > 5 | ||||||
|
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'd generally recommend raising errors like this before initializing fields. It doesn't matter here but can be helpful as constructors get more complex. Also, it's helpful to include the bad argument when you raise an
Suggested change
|
||||||
| end | ||||||
| end | ||||||
| end | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| module Hotel | ||
| class DateRange | ||
| attr_accessor :start_date, :end_date | ||
|
|
||
| def initialize(start_date:, end_date:) | ||
| @start_date = start_date | ||
| @end_date = end_date | ||
|
|
||
| raise ArgumentError.new("Invalid date range!") if @start_date > @end_date | ||
| end | ||
|
|
||
| def nights | ||
| return (@end_date - @start_date).to_i | ||
| end | ||
|
|
||
| def overlap?(other) | ||
| return @start_date <= other.end_date && @end_date >= other.start_date | ||
| end | ||
|
|
||
| def include?(date) | ||
| low = 0 | ||
| high = self.nights | ||
|
|
||
| while low <= high | ||
| mid = (low + high) / 2 | ||
| if (@start_date + mid) == date | ||
| return true | ||
| elsif (@start_date + mid) > date | ||
| high = mid - 1 | ||
| elsif (@start_date + mid) < date | ||
| low = mid + 1 | ||
| end | ||
| end | ||
|
|
||
| return false | ||
|
Comment on lines
+21
to
+35
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 cool but I'm pretty sure binary search is overkill here. I think you can just do something like |
||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| module Hotel | ||
| class FrontDesk | ||
| attr_reader :rooms, :reservations, :blocks | ||
|
|
||
| def initialize | ||
| @rooms = (1..20).to_a | ||
| @reservations = [] | ||
| @blocks = [] | ||
| end | ||
|
|
||
| def add_reservation(reservation) | ||
| @reservations << reservation | ||
| end | ||
|
|
||
| def reservations_by_room(room, date_range) | ||
| return @reservations.select { |reservation| | ||
| reservation.room == room && reservation.date_range.overlap?(date_range) | ||
| } | ||
| end | ||
|
|
||
| def reservations_by_date(date) | ||
| return @reservations.select { |reservation| | ||
| reservation.date_range.include?(date) | ||
| } | ||
| end | ||
|
|
||
| def find_available_room(date_range) | ||
| unavailable_rooms = @reservations.map { |reservation| | ||
| reservation.room if reservation.date_range.start_date < date_range.end_date && reservation.date_range.end_date > date_range.start_date | ||
| } | ||
| @blocks.each { |block| | ||
| (block.rooms).each { |room| unavailable_rooms << room } if block.date_range.start_date < date_range.end_date && block.date_range.end_date > date_range.start_date | ||
| } # This method is very expensive. See refactors.txt for details. | ||
| available_rooms = @rooms - unavailable_rooms | ||
|
|
||
| raise ArgumentError.new("No rooms available for that date range!") if available_rooms.empty? | ||
| return available_rooms | ||
| end | ||
|
Comment on lines
+27
to
+38
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 isn't as expensive as you think. Or at least it's not much worse than an optimal solution. For space complexity you can't do better than O(n) (because you might need to return an array of every room if they're all available). For time complexity it's a little trickier. Using a clever data structure you can get individual reservation comparison down to O(log(n)) but that still only gets you O(n*log(n)). If you use a data structure that O(r) where r is the number of days in all reservations then you can get the time down to O(rd) where d is the number of days in the input range but that uses a lot of space. Off the top of my head I can't think of a solution that performs well in both space and time so this overall seems pretty reasonable. Some operations are just inherently expensive no matter how well you code your solution. |
||
|
|
||
| def reserve_room(date_range, rate = 200.0) | ||
| new_reservation = Hotel::Reservation.new( | ||
| id: @reservations.length + 1, | ||
| room: find_available_room(date_range).first, | ||
| rate: rate.to_f, | ||
| start_date: date_range.start_date, | ||
| end_date: date_range.end_date | ||
|
Comment on lines
+45
to
+46
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 probably would have just stored the |
||
| ) | ||
| add_reservation(new_reservation) | ||
| return new_reservation | ||
| end | ||
|
|
||
| def add_block(block) | ||
| @blocks << block | ||
| end | ||
|
|
||
| def find_block(id) | ||
| block = @blocks.find { |block| block.id == id } | ||
| raise ArgumentError.new("No blocks with the given ID!") if block.nil? | ||
| return block | ||
| end | ||
|
|
||
| def reserve_block(rooms, rate, date_range) | ||
| rooms.each { |room| | ||
| raise ArgumentError.new("At least one of the rooms is unavailable for the given date range!") if !find_available_room(date_range).include?(room) | ||
| } | ||
|
|
||
| new_block = Hotel::Block.new( | ||
| id: @blocks.length + 1, | ||
| rooms: rooms, | ||
| rate: rate, | ||
| start_date: date_range.start_date, | ||
| end_date: date_range.end_date | ||
| ) | ||
| add_block(new_block) | ||
| return new_block | ||
| end | ||
|
|
||
| def find_available_room_in_block(id) | ||
| block = find_block(id) | ||
| unavailable_rooms = @reservations.map { |reservation| | ||
| reservation.room if reservation.block == block.id | ||
| } | ||
| available_rooms = block.rooms - unavailable_rooms | ||
|
|
||
| raise ArgumentError.new("No rooms available in the given block!") if available_rooms.empty? | ||
| return available_rooms | ||
| end | ||
|
|
||
| def reserve_room_in_block(id, room) | ||
| block = find_block(id) | ||
| new_reservation = Hotel::Reservation.new( | ||
| id: @reservations.length + 1, | ||
| room: room, | ||
| block: block.id, | ||
| rate: block.rate, | ||
| start_date: block.date_range.start_date, | ||
| end_date: block.date_range.end_date | ||
| ) | ||
| add_reservation(new_reservation) | ||
| return new_reservation | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,24 @@ | ||||||
| require_relative 'date_range' | ||||||
|
|
||||||
| module Hotel | ||||||
| class Reservation | ||||||
| attr_reader :id, :room, :block, :rate, :date_range | ||||||
|
|
||||||
| def initialize(id:, room:, block: false, rate: 200.0, start_date:, end_date:) | ||||||
| @id = id | ||||||
| @room = room | ||||||
| @block = block | ||||||
| @rate = rate.to_f | ||||||
| @date_range = Hotel::DateRange.new( | ||||||
| start_date: start_date, | ||||||
| end_date: end_date | ||||||
| ) | ||||||
|
|
||||||
| raise ArgumentError.new("Invalid rate!") if rate.class != Float | ||||||
|
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. If you're going to check types you should probably use
Suggested change
|
||||||
| end | ||||||
|
|
||||||
| def total_cost | ||||||
| return @date_range.nights * @rate | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| ================= | ||
| lib/front_desk.rb | ||
| ================= | ||
|
|
||
| 1) class FrontDesk | ||
| - Since the program will be used by employees of the hotel and not the general public, I thought FrontDesk would be more descriptive. But if the program is ported to an online reservation system that is open to the general public, the class name should probably change to HotelController or ReservationSystem. | ||
|
|
||
| 2) #find_available_room | ||
| - This method is very expensive because it iterates through two arrays of unknown size (time complexity is O(n^2)) and it creates a new array in memory each time it adds a room to the unavailable_rooms list (space complexity is O(n)). | ||
| - If I created a Room class that knew its Reservations and/or the Blocks it belonged to during a DateRange, I could refactor this method to just use the #reject enumerable, rejecting any rooms that met the existing conditionals. | ||
|
|
||
| 3) any method that has date_range as a parameter | ||
| - I was visualizing this program as the backend of a digital form with separate fields for the start and end dates of range (if the function required that information). Each time the 'submit' button is pressed, the program would collect the values of these fields and use them to instantiate a new DateRange object, then pass that object into the method. This is why the start_date and end_date attributes are both readable and writable, so they can be written from outside the program. However, I'm not sure if this will work as intended, so it's possible that many of my tests are passing superficially! | ||
|
|
||
| 4) #reserve_room & #reserve_room_in_block | ||
| - I may be able to merge these methods into one, because a Reservation can be instantiated without a block and rate attribute. | ||
| - If I create a Room class, I could have the rate stored in each Room instead of having it passed into these methods. | ||
| - But if the program is ported to an online reservation system that is open to the general public, these methods should stay separate because the general public is not allowed to reserve rooms in blocks. | ||
|
|
||
| ================== | ||
| lib/reservation.rb | ||
| ================== | ||
|
|
||
| 1) #rate & #total_cost | ||
| - If I create a Room class, I could have the rate stored in each Room instead of having it passed into a new Reservation. | ||
| - The #total_cost method would then call the room.rate | ||
|
|
||
| 2) #start_date & #end_date | ||
| - I was thinking that since these attributes can be written over in the DateRange class, they should probably not be stored anywhere else. But if each Reservation was able to call its start_date and end_date, this would simplify some method calls in the FrontDesk class. Perhaps this is an opportunity for inheritance (i.e., class Reservation < DateRange)? | ||
|
|
||
| ================= | ||
| lib/date_range.rb | ||
| ================= | ||
|
|
||
| 1) #include? | ||
| - An earlier version simply returned (start_date..end_date).include?(date), which creates an instance of the Range class, which already has an include? method. I'm wondering if this was a better design, but I also wanted to avoid dependencies (if the Range class changes in the future, any method that creates an instance of that class might break). Instead, I created a binary search method that is not dependent on an additional class. | ||
|
|
||
| ============ | ||
| lib/block.rb | ||
| ============ | ||
|
|
||
| 1) class Block | ||
| - If I create a Room class, there could be an opportunity for inheritance here (i.e., a Block is a Room), which could help with the FrontDesk#find_available_room (see notes above) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| require_relative 'test_helper' | ||
|
|
||
| describe "Block class" do | ||
| before do | ||
| @block = Hotel::Block.new( | ||
| id: 1, | ||
| rooms: (1..5).to_a, | ||
| rate: 150.0, | ||
| start_date: Date.new(2020,3,2), | ||
| end_date: Date.new(2020,3,5) | ||
| ) | ||
| end | ||
|
|
||
| describe "Block instantiation" do | ||
| it "is an instance of Block" do | ||
| expect(@block).must_be_kind_of Hotel::Block | ||
| end | ||
|
|
||
| it "is set up for specific attributes and data types" do | ||
| [:id, :rooms, :rate, :date_range].each do |attribute| | ||
| expect(@block).must_respond_to attribute | ||
| end | ||
|
|
||
| expect(@block.id).must_be_kind_of Integer | ||
| expect(@block.rooms).must_be_kind_of Array | ||
| expect(@block.rate).must_be_kind_of Float | ||
| expect(@block.date_range).must_be_kind_of Hotel::DateRange | ||
| end | ||
|
Comment on lines
+19
to
+28
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. Something confusing happened with your indentation here. |
||
|
|
||
| it "throws an exception if the list of rooms is greater than 5" do | ||
| expect{ Hotel::Block.new( | ||
| id: 1, | ||
| rooms: (1..6).to_a, | ||
| rate: 150.0, | ||
| start_date: Date.new(2020,3,2), | ||
| end_date: Date.new(2020,3,5) | ||
| ) }.must_raise ArgumentError | ||
| end | ||
|
|
||
| it "throws an exception if the list of rooms is less than 2" do | ||
| expect{ Hotel::Block.new( | ||
| id: 1, | ||
| rooms: [1], | ||
| rate: 150.0, | ||
| start_date: Date.new(2020,3,2), | ||
| end_date: Date.new(2020,3,5) | ||
| ) }.must_raise ArgumentError | ||
| 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.
You can ignore these everywhere by creating a global gitignore file.
https://help.github.com/en/github/using-git/ignoring-files#configuring-ignored-files-for-all-repositories-on-your-computer