goeun - edges - hotel #22

goeunpark · 2018-09-10T06:40:42Z

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
Describe a design decision you had to make when working on this project. What options were you considering? What helped you make your final decision?	I initially over-designed and my classes had too much dependency (rooms knew which reservations they were associated with) and I thought I would create a lot more data structures than necessary (a whole array in `HotelBooker` for all the dates ever booked). Thinking about how Chapter 3 Sandi Metz would be unimpressed with my agenda helped. Having a looming deadline made me feel rushed so I went with options that felt like it would let me get away with bare minimum. In retrospect, that was a pretty good design plan.
Describe a concept that you gained more clarity on as you worked on this assignment.	`HotelBooker` is my factory class. Initially, I had `Reservation`s create `Room`s and `Room`s refer to `DateRange`s but then it got too messy so I put all the logic into my main class. I see now that `TripDispatcher` in Rideshare was set up to instantiate all the objects for a reason!
Describe a nominal test that you wrote for this assignment.	I created one reservation for one date range (lines 32 - 41 in `HotelBooker`). Easy. Things seemed to work.
Describe an edge case test that you wrote for this assignment.	I created 20 reservations for one date range (lines 43 - 49 in `HotelBooker`). Then I tried to create one more. But there were no more available rooms, so I raised an `StandardError`
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	Feel pretty good about writing pseudocode! I feel like I was A LOT better at git hygiene and TDD with a partner. Will work on it

…d room

…, and can get total_cost of reservation

…t; test for find_reservations(date) in hotel_booker is red

tildeee · 2018-09-21T06:11:28Z

Hotel

What We're Looking For

Feature	Feedback
Baseline
Used git regularly	x
Answer comprehension questions	x
Design
Each class is responsible for a single piece of the program	x
Classes are loosely coupled	x
Wave 1
List rooms	x
Reserve a room for a given date range	x
List reservations for a given date	x
Calculate reservation price	x
Invalid date range produces an error	x
Test coverage	x
Wave 2
View available rooms for a given date range	x
Reserving a room that is not available produces an error	x
Test coverage	x
Wave 3
Create a block of rooms	x
Check if a block has rooms	x
Reserve a room from a block	x
Test coverage	x
Fundamentals
Names variables, classes and modules appropriately	x, fantastic!
Understanding of variable scope - local vs instance	x
Can create complex logical structures utilizing variables	x
Appropriately uses methods to break down tasks into smaller simpler tasks	x
Understands the differences between class and instance methods	x
Appropriately uses iterators and enumerables	x
Appropriately writes and utilizes classes	x
Appropriately utilizes modules as a mechanism to encapsulate functionality	x
Wrap Up
There is a refactors.txt file	x
The file provides a roadmap to future changes	x
Additional Feedback

Goeun! Good job with this project!

I think that your code looks good-- I think that your design is pretty good-- your HotelBooker manages the reservations, rooms, reserved blocks, and unreserved blocks. The complex date overlap logic lives in the DateRange class, which I think really paid off.

Overall, my biggest comment is that I truly feel that the tests were thorough, clean, and readable. I in fact have very little comments on the tests because I think they're great-- wish I had more to comment on, but I really think you did a great job on the tests!

That being said, I'm adding some comments on specific pieces of code. The main themes on these comments are:

Suggestions for minor refactorings to clean up code style
Weirdly enough, I think you're pretty inconsistent with checking if an array is empty, so I'm calling that out a little specifically??

In conclusion: generally good job! I think I could see your code being a tiny bit cleaner, but the DateRange decision and the tests you wrote are really great and overwhelmingly positive.

tildeee · 2018-09-21T06:12:24Z

lib/hotel_booker.rb

+  class HotelBooker
+    attr_accessor :rooms, :reservations, :unreserved_block, :reserved_block
+    NUM_ROOMS = 20
+    BLOCK_MAX = 5


YES CONSTANTS

tildeee · 2018-09-21T06:13:08Z

lib/hotel_booker.rb

+      @rooms = []
+
+      NUM_ROOMS.times do |i|
+        @rooms << Hotel::Room.new(i+1)


Ruby will be looking for names locally first, prioritizing local names... so, since they're all in the same module, there's no need to namespace Hotel::Room... Room should just work! Same with the rest of the namespacing in the code.

tildeee · 2018-09-21T06:13:32Z

lib/hotel_booker.rb

+      check_in = Date.parse(check_in)
+      check_out = Date.parse(check_out)
+      date_range = range(check_in, check_out)
+      reservation = Hotel::Reservation.new(id, date_range)


It might give you less "overusing the word range"-anxiety to in-line this:

reservation = Hotel::Reservation.new(id, range(check_in, check_out))

tildeee · 2018-09-21T06:13:51Z

lib/hotel_booker.rb

+      available = unreserved_rooms(check_in, check_out)
+      if available == []
+        raise StandardError, "There are no more available rooms for this date range!"
+      end


It probably reads better to write this as if available.empty?, or maybe inline it somehow ;)

tildeee · 2018-09-21T06:14:08Z

lib/hotel_booker.rb

+    end
+
+
+    def make_block(rooms: nil, discount: nil, check_in: nil, check_out: nil)


This keyword args syntax makes these parameters optional: is that what you want?

tildeee · 2018-09-21T06:14:39Z

lib/hotel_booker.rb

+    def make_block_reservation(id)
+      if @unreserved_block.length ==  0
+        raise StandardError, "There are no available Block reservations."
+      end


I think this is another block of code that checks if an array is empty in a different way? If so -- check my comment above!

tildeee · 2018-09-21T06:15:03Z

lib/hotel_booker.rb

+      unreserved_block_rooms = []
+      @unreserved_block.each do |reservation|
+        unreserved_block_rooms << reservation.room
+      end


Could possibly be a good chance to use map

tildeee · 2018-09-21T06:15:18Z

lib/hotel_booker.rb

+        if reservation_dates.include?(date)
+          matching_reservations << reservation
+        end
+      end


Instead of using .each, might be good opportunity to use select

tildeee · 2018-09-21T06:15:32Z

refactors.txt

@@ -0,0 +1,7 @@
+1. I...lowkey cheated and made a collection of Reservations instead of a collection of Rooms. I would rewrite this part by making a subclass under Room called BookedRoom?


Not really sure what you mean by this tbh!

tildeee · 2018-09-21T06:15:41Z

refactors.txt

@@ -0,0 +1,7 @@
+1. I...lowkey cheated and made a collection of Reservations instead of a collection of Rooms. I would rewrite this part by making a subclass under Room called BookedRoom?
+
+2. HotelBooker has a lot of WET code because I have to loop through several data structures (@unreserved_block, @reserved_block, @reservations)!! If I had focused the Wave 3 blocks of rooms on the ROOMS instead of reservations, probably wouldn't have created this mess ¯\_(ツ)_/¯


tildeee · 2018-09-21T06:15:53Z

refactors.txt

+
+2. HotelBooker has a lot of WET code because I have to loop through several data structures (@unreserved_block, @reserved_block, @reservations)!! If I had focused the Wave 3 blocks of rooms on the ROOMS instead of reservations, probably wouldn't have created this mess ¯\_(ツ)_/¯
+
+3. there are certain check_in and check_out arguments that are strings (as in Hotel::HotelBooker.new.make_reservation) and some that are an instance of Date (as in Hotel::HotelBooker.new.unreserved_rooms). Other arguments pass in a Hotel::DateRange (such as Hotel::Reservations). note to future goeun: make this less terrible


Would absolutely agree with this

tildeee · 2018-09-21T06:16:12Z

spec/date_range_spec.rb

+      expect(@date_range2.overlaps?(@date_range4)).must_equal false
+    end
+  end
+end


I love this DateRange class because it isolated the overlapping date logic to this one class, which is complex and tested in isolation... Beautiful :* truly inspiring :*

goeunpark added 23 commits September 4, 2018 17:00

create spec and lib files for hotel booker, calendar, reservation, an…

a79e8c4

…d room

HotelBooker initates 20 rooms, can list all rooms, can reserve a room…

73609dc

…, and can get total_cost of reservation

renamed booked_dates to date_range, started spec and method calls

44059e3

overlap? method in date_range returns boolean if two date ranges overlap

d0ba0bf

overlaps? method in date_range returns boolean

d4e5b6d

test for raising standard error for invalid date ranges is red

62b0679

raises standard error for invalid date ranges: green

b054817

raises ArgumentError if dates argument are not Dates

aaedaaf

change arguments in reservation to date_range, not check_in, check_ou…

fe5d223

…t; test for find_reservations(date) in hotel_booker is red

find_reservation in hotel_booker passes tests

bfbb588

split up tests for overlaps? in date_range

fe6c4f2

unreserved_rooms method in hotel_booker failes test

caaf57b

initial pass for unreserved_rooms method in hotel_booker

c0cee89

clean up Room and room_spec files

b0fc136

cleaned up Reservation and Reservation_spec files

a431409

updated unreserved_rooms method testing in hotel_booker_spec

4859efe

start tests for create_block method in hotel_booker

e8de7fe

hotel_booker can create a block of rooms

4f89d83

reservation dates match the date range of the block test passes

abeee86

start make_block_reservation method

5a90808

can book a block reservation

c4d7e68

create unreserved_block_rooms method and passing test

a963903

add a refactors file

e781d16

tildeee reviewed Sep 21, 2018

View reviewed changes

goeunpark added 7 commits September 30, 2018 15:32

remove unnecessary namespacing

eca8b38

refactor checking available rooms into its own method

1be5ba3

start design-activity responses

8278260

add bolded questions

108afe7

finish design-activity questions

df6bd13

refactor unreserved_block_rooms method

821a8db

refactor / design in changing cost of room when creating blocks

96f44ed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

goeun - edges - hotel #22

goeun - edges - hotel #22

goeunpark commented Sep 10, 2018

tildeee commented Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

tildeee Sep 21, 2018

		end


		def make_block(rooms: nil, discount: nil, check_in: nil, check_out: nil)

		@@ -0,0 +1,7 @@
		1. I...lowkey cheated and made a collection of Reservations instead of a collection of Rooms. I would rewrite this part by making a subclass under Room called BookedRoom?

		@@ -0,0 +1,7 @@
		1. I...lowkey cheated and made a collection of Reservations instead of a collection of Rooms. I would rewrite this part by making a subclass under Room called BookedRoom?

		2. HotelBooker has a lot of WET code because I have to loop through several data structures (@unreserved_block, @reserved_block, @reservations)!! If I had focused the Wave 3 blocks of rooms on the ROOMS instead of reservations, probably wouldn't have created this mess ¯\_(ツ)_/¯


		2. HotelBooker has a lot of WET code because I have to loop through several data structures (@unreserved_block, @reserved_block, @reservations)!! If I had focused the Wave 3 blocks of rooms on the ROOMS instead of reservations, probably wouldn't have created this mess ¯\_(ツ)_/¯

		3. there are certain check_in and check_out arguments that are strings (as in Hotel::HotelBooker.new.make_reservation) and some that are an instance of Date (as in Hotel::HotelBooker.new.unreserved_rooms). Other arguments pass in a Hotel::DateRange (such as Hotel::Reservations). note to future goeun: make this less terrible

goeun - edges - hotel #22

Are you sure you want to change the base?

goeun - edges - hotel #22

Conversation

goeunpark commented Sep 10, 2018

Hotel

Comprehension Questions

tildeee commented Sep 21, 2018

Hotel

What We're Looking For

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment