Space - Hala #30

halahaddad1 · 2020-03-09T14:14:10Z

Assignment Submission: Hotel

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

Reflection

Question	Answer
What was a design challenge that you encountered on this project?	Figuring out my class structure, and what minimum attributes were actually needed
What was a design decision you made that changed over time over the project?	I had a room status in my rooms class attributes, I discovered it was useless down the line and deleted it! also I originally had a list of reservations in my hotel_manager class, and later discovered it was not needed!
What was a concept you gained clarity on, or a learning that you'd like to share?	I definitely got clarity on how classes interact, and how to construct a driver class
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case?	Testing number of rooms in block were exactly 4, another example is testing reservations in rooms, etc. and it is nominal because it expects a certain value returned
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case?	A frequent edge case I used was checking for date overlaps on the same date of the checkout, also beginning and ends of date ranges
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	I did very in writing pseudocode at the beginning, when I was still figuring out my program structure. as I started to construct my classes I started to spend less time on pseudocode.

…ms into that class

dHelmgren · 2020-03-22T21:52:08Z

Hotel

Section 1: Major Learning Goals

Criteria	yes/no, and optionally any details/lines of code to reference
Practices SRP by having at least two separate classes with distinct responsibilities, and test files for these two classes	✔️
Overall, demonstrates understanding instance variables vs. local variables. (There aren't unnecessarily too many instance variables, when it should be a local variable)	✔️
For each test file, tests demonstrate an understanding of instantiating objects properly, and using Arrange-Act-Assert	✔️
Practices pseudocode and TDD, and reflected on it by filling out the reflection questions	✔️
Practices git with at least 15 small commits and meaningful commit messages

Section 2: Code Review and Testing Requirements

Criteria	yes/no, and optionally any details/lines of code to reference
There is a class that represents a reservation, and a second class that holds/manages a collection of reservations through composition (instance variable)	✔️
The logic for checking if a reservation's date overlaps with another reservation's date is complex logic that is separated into method(s) (and potentially class(es))	✔️
The logic for checking if a reservation's date overlaps with another reservation's date has unit tests
All of the given tests run and pass	✔️
A test coverage tool is installed and used, and shows 95% test coverage	✔️

Section 3: Feature Requirements

Feature Requirement: There is a method that...	yes/no
gives back a list of rooms, and it's tested	✔️
creates a specific reservation for a room for a given date range, and it has nominal test cases	✔️
creates a specific reservation for a room for a given date range, and it tests an edge case, such as no available room, or invalid date range	✔️
gives back a list of reservations on a given date. Its tests include a test that makes several reservations for a given date	✔️
calculates the total price for a reservation	✔️
gives back a list of available rooms for a given date range, and it has nominal test cases	✔️
gives back a list of available rooms for a given date range, and it has edge test cases, such as no available room, or invalid date range	✔️
creates a block of rooms
reserves a room from a block

Overall Feedback

Overall Feedback	Criteria	yes/no
Green (Meets/Exceeds Standards)	14+ total in all sections
Yellow (Approaches Standards)	9-13 total in all sections	✔️
Red (Not at Standard)	0-8 total in all sections, or assignment is breaking/doesn’t run with less than 5 minutes of debugging

Additional Feedback

Great work overall! You've built your first project with minimal starting code. This represents an incredible milestone in your journey, and you should be proud of yourself!

I am particularly impressed by the way that you broke down the problem in readable methods that each dealt with a small piece of the larger problems. Doing this effectively not only helps you read your code more effectively, but the way in which your methods are reusable makes it so that your code can flex and adapt to the future.

I do see some room for improvement around testing and responsibility separations. Your tests cover a lot of good ground, but especially with something like checking if date ranges overlap, you need to be exhaustive in your testing. I often draw a picture or make a table to make sure I'm not missing anything key. Also, I left a few comments in your code about how responsibilities are divvied up. Basically, we want to make sure that our classes exist not only to hold their data, but to answer questions about it and manage it.

Your code produced the following warnings, among others:

/Users/devin/Documents/Ada/c13/hotel/test/hotel_block_test.rb:11: warning: assigned but unused variable - date_in2
/Users/devin/Documents/Ada/c13/hotel/test/hotel_block_test.rb:12: warning: assigned but unused variable - date_out2
/Users/devin/Documents/Ada/c13/hotel/test/hotel_block_test.rb:13: warning: assigned but unused variable - date_in3
/Users/devin/Documents/Ada/c13/hotel/test/hotel_block_test.rb:14: warning: assigned but unused variable - date_out3
/Users/devin/Documents/Ada/c13/hotel/lib/reservations.rb:2: warning: loading in progress, circular require considered harmful - /Users/devin/Documents/Ada/c13/hotel/lib/room.rb
    from /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/rake-13.0.0/lib/rake/rake_test_loader.rb:5:in  `<main>'
    from /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/rake-13.0.0/lib/rake/rake_test_loader.rb:5:in  `select'
    from /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/rake-13.0.0/lib/rake/rake_test_loader.rb:17:in  `block in <main>'
    from /Users/devin/.rbenv/versions/2.5.5/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in  `require'
    from /Users/devin/.rbenv/versions/2.5.5/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in  `require'
    from /Users/devin/Documents/Ada/c13/hotel/test/hotel_block_test.rb:1:in  `<top (required)>'
    from /Users/devin/Documents/Ada/c13/hotel/test/hotel_block_test.rb:1:in  `require_relative'
    from /Users/devin/Documents/Ada/c13/hotel/test/test_helper.rb:19:in  `<top (required)>'
    from /Users/devin/Documents/Ada/c13/hotel/test/test_helper.rb:19:in  `require_relative'
    from /Users/devin/Documents/Ada/c13/hotel/lib/hotel_manager.rb:2:in  `<top (required)>'
    from /Users/devin/Documents/Ada/c13/hotel/lib/hotel_manager.rb:2:in  `require_relative'
    from /Users/devin/Documents/Ada/c13/hotel/lib/room.rb:1:in  `<top (required)>'
    from /Users/devin/Documents/Ada/c13/hotel/lib/room.rb:1:in  `require_relative'
    from /Users/devin/Documents/Ada/c13/hotel/lib/reservations.rb:2:in  `<top (required)>'
    from /Users/devin/Documents/Ada/c13/hotel/lib/reservations.rb:2:in  `require_relative'
/Users/devin/Documents/Ada/c13/hotel/lib/reservations.rb:3: warning: loading in progress, circular require considered harmful - /Users/devin/Documents/Ada/c13/hotel/lib/hotel_manager.rb
    from /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/rake-13.0.0/lib/rake/rake_test_loader.rb:5:in  `<main>'
    from /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/rake-13.0.0/lib/rake/rake_test_loader.rb:5:in  `select'
    from /Users/devin/.rbenv/versions/2.5.5/lib/ruby/gems/2.5.0/gems/rake-13.0.0/lib/rake/rake_test_loader.rb:17:in  `block in <main>'
    from /Users/devin/.rbenv/versions/2.5.5/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in  `require'
    from /Users/devin/.rbenv/versions/2.5.5/lib/ruby/2.5.0/rubygems/core_ext/kernel_require.rb:59:in  `require'
    from /Users/devin/Documents/Ada/c13/hotel/test/hotel_block_test.rb:1:in  `<top (required)>'
    from /Users/devin/Documents/Ada/c13/hotel/test/hotel_block_test.rb:1:in  `require_relative'
    from /Users/devin/Documents/Ada/c13/hotel/test/test_helper.rb:19:in  `<top (required)>'
    from /Users/devin/Documents/Ada/c13/hotel/test/test_helper.rb:19:in  `require_relative'
    from /Users/devin/Documents/Ada/c13/hotel/lib/hotel_manager.rb:2:in  `<top (required)>'
    from /Users/devin/Documents/Ada/c13/hotel/lib/hotel_manager.rb:2:in  `require_relative'
    from /Users/devin/Documents/Ada/c13/hotel/lib/room.rb:1:in  `<top (required)>'
    from /Users/devin/Documents/Ada/c13/hotel/lib/room.rb:1:in  `require_relative'
    from /Users/devin/Documents/Ada/c13/hotel/lib/reservations.rb:3:in  `<top (required)>'
    from /Users/devin/Documents/Ada/c13/hotel/lib/reservations.rb:3:in  `require_relative'
/Users/devin/Documents/Ada/c13/hotel/lib/hotel_block.rb:34: warning: mismatched indentations at 'end' with 'class' at 6

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality	Yes?
Perfect Indentation	✅
Descriptive/Readable	✅

dHelmgren · 2020-03-22T20:45:43Z

lib/hotel_block.rb

+    # def room_aside_for_date_range(start_date,end_date,room)
+    #   range = date_in...date_out
+    #   rooms_block.each do |room_in_block| 
+    #     if room_in_block == room
+    #       if range.include?(date)
+    #         return :UNAVAILABLE
+    #       end
+    #     end
+    #   end
+    #   else return :AVAILABLE
+    # end


Don't forget to remove dead code before submission! As long as you have committed the code you can get old methods like this back.

dHelmgren · 2020-03-22T20:58:20Z

lib/hotel_manager.rb

+      end
+    end
+
+    def add_a_reservation_to_room(room, reservation)


I would define this inside of Room, because managing a class's data makes the most sense as a responsibility of that class. Also, it removes bloat from your HotelManager.

dHelmgren · 2020-03-22T21:00:29Z

lib/hotel_manager.rb

+    def add_rooms_to_block(hotelblock,number_of_rooms_needed)
+      number_of_available_rooms = 0
+      rooms.each do |room|
+        if room.test_overlap(hotelblock.date_in,hotelblock.date_out) == :AVAILABLE 
+          number_of_available_rooms +=1
+        end
+      end
+      if number_of_available_rooms < number_of_rooms_needed
+        raise ArgumentError, "Not enough room AVAILABILITY for this Block! Only #{number_of_available_rooms} AVAILABLE"
+      else
+        rooms_added = 0
+        rooms.each do |room| 
+          if room.test_overlap(hotelblock.date_in,hotelblock.date_out) == :AVAILABLE 
+          hotelblock.rooms_block<< room
+          rooms_added +=1
+          if rooms_added == number_of_rooms_needed
+            return
+          end
+          end
+        end
+      end
+    end


Again, this is probably the responsibility of HotelBlock, as it changes data inside the Block.

dHelmgren · 2020-03-22T21:01:33Z

lib/hotel_block.rb

+    class HotelBlock
+        attr_accessor :date_in, :date_out, :number_of_rooms, :rooms_block
+
+        @@increment_id = 1
+      def initialize(date_in:,date_out:,number_of_rooms:, rooms_block:[], discounted_room_rate:)
+        @id = @@increment_id 
+        @@increment_id += 1
+        @date_in = date_in
+        @date_out = date_out
+        @number_of_rooms = number_of_rooms
+        # this range does not work, its an array of numbers
+        #raise argument error unless (2..5).include? number_of_rooms 
+        @rooms_block =[]
+      end


I always get suspicious if a class ends up with just initialize. It usually means that the class in question is having its methods 'stolen' by other classes.

dHelmgren · 2020-03-22T21:03:08Z

lib/reservations.rb

+
+    attr_accessor :id, :date_in, :date_out, :room_id
+
+    @@increment_id = 1
+    def initialize(date_in:, date_out:, room_id: , room: )
+      @id = @@increment_id
+      @@increment_id += 1
+      @date_in = date_in
+      @date_out = date_out
+      @room_id = room_id
+      @room = room
+
+      unless date_out >= date_in 
+        raise ArgumentError, 'you cannot have date out come before date in'
+      end 
+
+      if room.test_overlap(date_in,date_out) == :UNAVAILABLE
+          raise ArgumentError, 'there is an overlap'
+      end  
+    end
+
+    def total_number_of_nights_per_reservation
+      return  date_out - date_in 
+    end
+


This class does a great job knowing what it's responsible for and having a good selection of methods for returning important info.

dHelmgren · 2020-03-22T21:05:32Z

lib/room.rb

+          overlap1 = start_date >= reservation.date_in && start_date < reservation.date_out
+          overlap2 = end_date > reservation.date_in && end_date < reservation.date_out 
+          overlap3 = start_date >= reservation.date_in && end_date < reservation.date_out
+          overlap4 = start_date <= reservation.date_in && end_date >= reservation.date_out
+
+          overlap1 || overlap2 || overlap3 || overlap4


There is a simpler way of doing this date checking! Make a timeline, put an existing reservation on the timeline, and then see where you can draw lines that don't conflict!

dHelmgren · 2020-03-22T21:19:53Z

lib/hotel_manager.rb

+    attr_accessor :rooms, :blocks
+
+    def initialize(rooms:,blocks:)
+      @rooms = @Hotel::Room.list_of_rooms


When you are inside the scope of module Hotel you don't need to do Hotel::. The @Hotel ends up causing a bunch of warnings.

dHelmgren · 2020-03-22T21:42:51Z

test/room_test.rb

+        date_in1 = DateTime.new(2020,2,3.5)
+        date_out1 = DateTime.new(2020,2,5.5)
+        date_in2 = DateTime.new(2020,2,7.5)
+        date_out2 = DateTime.new(2020,2,14.5)
+        date_in3 = DateTime.new(2020,2,14.5)
+        date_out3 = DateTime.new(2020,2,16.5)
+        @hotel = Hotel::HotelManager.new(rooms: Hotel::Room.list_of_rooms,blocks: [])
+        @reservation1 = Hotel::Reservations.new(date_in:date_in1 , date_out:date_out1, room_id:1, room: @hotel.rooms[0])
+        @reservation2 = Hotel::Reservations.new(date_in:date_in2 , date_out:date_out2, room_id:1, room: @hotel.rooms[0])
+        @reservation3 = Hotel::Reservations.new(date_in:date_in3 , date_out:date_out3, room_id:1, room: @hotel.rooms[0])
+        @hotel.add_a_reservation_to_room(@hotel.rooms[0],@reservation1)
+        @hotel.add_a_reservation_to_room(@hotel.rooms[0],@reservation2)
+        @hotel.add_a_reservation_to_room(@hotel.rooms[0],@reservation3)
+        @room = Hotel::Room.new(id: 34, reservations: [])


Do you need all of this data for every single test?

dHelmgren · 2020-03-22T21:44:45Z

test/room_test.rb

+    it "returns false when a room is anavailable" do
+        date = DateTime.new(2020,2,4.5)
+        date1 = DateTime.new(2020,3,4.5)
+        date2 = DateTime.new(2020,2,5.5)
+        date3 = DateTime.new(2020,2,14.5)
+        expect(@hotel.rooms[0].specific_date_availability(date)).must_equal [false]
+        expect(@hotel.rooms[0].specific_date_availability(date1)).must_equal []
+        expect(@hotel.rooms[0].specific_date_availability(date2)).must_equal []
+        expect(@hotel.rooms[0].specific_date_availability(date3)).must_equal [false]
+    end


There are a lot of ways that date ranges can overlap! Somewhere you need to exhaustively test them to make sure your date checking logic is correct.

dHelmgren · 2020-03-22T21:46:52Z

test/hotel_block_test.rb

+      @block = Hotel::HotelBlock.new(date_in: date_in1,date_out: date_out1 ,number_of_rooms: (2..5), rooms_block:[],discounted_room_rate: 150)
+    end
+
+    it "is an instance of FrontDesk" do


halahaddad1 added 8 commits March 2, 2020 14:20

Got first green test on class FrontDesk

e74153e

room reservations and hotel manager classes initialized and tested

9d8e6f8

added list_all_rooms method to rooms and hotel classes

126387f

wave 2 done and tested

293d4b8

wrote hotel block class and successfully wrote a method that adds roo…

1ccf2b7

…ms into that class

wave 3 almost done! all code and tests working!

2b613ba

wave 3 done and fully tested

fa93cdb

added refactors.txt file

a077e6d

halahaddad1 changed the title ~~Hotel: Hala~~ Space - Hala Mar 9, 2020

dHelmgren reviewed Mar 22, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Space - Hala #30

Space - Hala #30

Uh oh!

halahaddad1 commented Mar 9, 2020

Uh oh!

dHelmgren commented Mar 22, 2020

Uh oh!

dHelmgren Mar 22, 2020

Uh oh!

dHelmgren Mar 22, 2020

Uh oh!

dHelmgren Mar 22, 2020

Uh oh!

dHelmgren Mar 22, 2020

Uh oh!

dHelmgren Mar 22, 2020

Uh oh!

dHelmgren Mar 22, 2020

Uh oh!

dHelmgren Mar 22, 2020

Uh oh!

dHelmgren Mar 22, 2020

Uh oh!

dHelmgren Mar 22, 2020

Uh oh!

dHelmgren Mar 22, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Space - Hala #30

Are you sure you want to change the base?

Space - Hala #30

Uh oh!

Conversation

halahaddad1 commented Mar 9, 2020

Assignment Submission: Hotel

Reflection

Uh oh!

dHelmgren commented Mar 22, 2020

Hotel

Section 1: Major Learning Goals

Section 2: Code Review and Testing Requirements

Section 3: Feature Requirements

Overall Feedback

Additional Feedback

Code Style Bonus Awards

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants