Pipes - Angela- Hotel #45

awilson2017 · 2017-09-11T15:58:43Z

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?	There were many design decision. The first was choosing how many classes I wanted within the module. I chose 3 with Admin class storing the daily functions of a hotel. The next major decision was accepting blocks. I decided to have Blocks Class dual as the reservation function. This was a mistake as I ran into many stumbling blocks and was not able to course correct before the deadline.
Describe a concept that you gained more clarity on as you worked on this assignment.	I gained clarity on how classes interact with each other. Plus overall design. My ability to take in a large amount of requirements at once and then parse out small manageable tasks will get better in time. Utilizing Git is something I really internalized. For this project I underutilized it and found I wanted to look at code I wrote that was no longer on my computer.
Describe a nominal test that you wrote for this assignment.	Creating a list of rooms. I tested for rooms 1 and 20.
Describe an edge case test that you wrote for this assignment.	Creating a list of rooms. I tested for rooms 0 and 21.
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	I felt okay writing psuedocode. As for writing test and then code, I found with challenging portions of the code, I went straight for coding and then writing the test. Rushing to get the logic out of my mind and onto the screen was important, otherwise I would have forgotten quickly. Overall, I consolidated my learning with this assignment and felt like the program could be improve. My assignment is incomplete and the portion I did complete is squirrelly and so I want to sit with someone to go over and specifics to this assignment. It would also be nice to discuss overarching strategies to improve.

…ers passed to Room instances.

Write initialize test for DateRange class and write DateRange implimintation code. Rename Hotel class to Admin class. Stored unused work in unused-work folder for potential future use.

Made tests for reservation class' added methods. Starting to work on Admin class and tests.

…tion(date), and list_vacancies. Need to write tests for almost all the listed methods.

So specs file was not staged. Specs are written for code except list_of_rooms, add_reservation, list_reservation(date), and list_vacancies methods.

…rvations.

…vation functions. This commit, refactor the initialize method to take a discount and refactor total_cost method to take discount.

…able_blocked_rooms, and find_rooms_from_block. Also added method add_reservation_to_block.

…block refactor. Work on refactoring the specs for block code.

droberts-sea · 2017-09-12T18:18:57Z

Hotel

What We're Looking For

Feature	Feedback
Baseline
Used git regularly	yes - From your comprehension Qs, it sounds like you weren't happy with your git habits. I like your commit messages, but I think I agree that more frequent commits would be helpful. As far as keeping track of old work attempts, this is a problem solved by branching, a topic you're welcome to go research now but which we'll cover formally in a few weeks.
Answer comprehension questions	yes
Design
Each class is responsible for a single piece of the program	some - see inline comment re: `DateRange`
Classes are loosely coupled	some - I've identified places inline where tight coupling occurs
Wave 1
List rooms	yes
Reserve a room for a given date range	some - see inline comment
List reservations for a given date	yes
Calculate reservation price	This is commented out? Not sure why.
Invalid date range produces an error	yes
Test coverage	some - reservation tests are missing, tests for `Admin`
Wave 2
View available rooms for a given date range	yes
Reserving a room that is not available produces an error	no
Test coverage	some - see inline
Wave 3
Create a block of rooms	yes
Check if a block has rooms	I can see how this would be done given existing methods, but there is not an explicit method for this
Reserve a room from a block	yes
Test coverage	some - see inline
Additional Feedback
File Names	The ruby standard is that files should be named using snake_case - all lowercase, underscores between words. So instead of `lib/DateRange.rb` it would be `lib/date_range.rb`.
Test Coverage	In general, your test coverage is not where I'd like it to be. There are many places where you miss interesting test cases and interactions between pieces of your code, which I've tried to call out below. Writing good test cases is not easy, it requires you to have a strong understanding of how your method will flow, how it will be used and what sorts of things might be issues, but that's precisely why we teach it. Please spend some time studying up on testing, and feel free to reach out to us if you want to work one-on-one with someone on this or need some extra problems to work on.
General Vibe	This is a good start, but it feels like there's a way to go still. The general pattern I notice is that your specific code to solve a problem is usually solid, but design and test coverage could be improved. To me, this indicates you're having trouble taking a step back and imagining what your code looks like from the outside. This is an important and difficult skill, and it's one you can grow with deliberate practice.

droberts-sea · 2017-09-11T22:08:20Z

lib/Block.rb

+      @discount_percent = discount_percent
+
+
+      @reservations_array2 = []


Why is this called reservations_array2? In general, if you need to put a number after a variable's name that's usually a bad sign. Moreover, you don't have a reservations_array1 that would conflict with it.

Note that instance variables in different classes (here vs Hotel) can have the same name - that's one of the big benefits of classes, is that they keep such things separated.

droberts-sea · 2017-09-11T22:09:47Z

lib/Admin.rb

+
+      @blocks.each do |block|
+        block.reservations_array2.each do |reservation|
+          rez_by_date << reservation


Won't this add all reservations from all blocks to the list, regardless of whether the date is included?

droberts-sea · 2017-09-11T22:14:14Z

lib/Admin.rb

+      available_rooms = list_of_rooms
+
+      date_range = DateRange.new(check_in, check_out).date_range_array
+


All the places that you create a DateRange, it seems like you throw away the actual DateRange instance and only keep track of the result of calling date_range_array. This indicates that a method might be a better choice here, one that takes a checkin and checkout date and returns an array of dates.

The other option would be to write an instance method, something like DateRange#overlap?(other), that takes another DateRange and checks to see if they overlap. Then this complex bit of logic would be hidden away in DateRange instead of sitting here in Hotel.

As long as you've identified "manage a range of dates" as a responsibility, then it probably makes sense to do the later, and keep that responsibility out of the Hotel class.

This would also make it much easier to test!

droberts-sea · 2017-09-11T22:36:10Z

lib/Admin.rb

+
+      num_rooms_to_reserve.length.times do |room_num|
+        block.reservations_array << Hotel::Reservation.new(room_num, check_in, check_out, discount_percent: 0)
+      end


This is an example of tight coupling between Hotel and Block. This method relies on the block having a reservations_array, which means if that implementation ever changes this code will need to change too. Instead, it might make sense to write a reserve_room method on the Block class and call that here.

droberts-sea · 2017-09-11T22:46:30Z

lib/Admin.rb

+    def create_block_by_date(rooms_per_block, check_in, check_out, block_id, discount_percent: 0.0)
+      room_num_array = self.list_vacancies(check_in, check_out).take(rooms_per_block).to_a
+      @blocks << Hotel::Block.new(room_num_array, check_in, check_out, block_id, discount_percent: 0.0)
+    end


I'm not sure this will pass the discount_percent through correctly - did you test this?

droberts-sea · 2017-09-11T23:33:04Z

specs/admin_spec.rb

+
+
+        id = 0
+        @admin.find_block(id).wont_be_kind_of Hotel::Block


So what will it be? nil?

droberts-sea · 2017-09-11T23:34:24Z

specs/date_range_spec.rb

+
+    it "Returns array of dates in range" do
+      @date_range.date_range_array.must_be_instance_of Array
+    end


You should explicitly check that the dates are as expected.

droberts-sea · 2017-09-11T23:44:14Z

specs/admin_spec.rb

+  before do
+
+    @admin = Hotel::Admin.new
+


I like the idea of DRYing all this up with a before block, since it's used in most of your tests.

droberts-sea · 2017-09-11T23:48:05Z

specs/admin_spec.rb

+      @admin.blocks.length.must_equal 1
+
+      # @admin.create_block_by_date(rooms_per_block, check_in, check_out, 2).total_cost.must_equal 200
+      # @admin.create_block_by_date(rooms_per_block, check_in, check_out, 2).wont_equal 0


Can you create multiple blocks that share the same dates? What if there are no rooms left for that date?

droberts-sea · 2017-09-11T23:50:05Z

lib/Admin.rb

+
+    def add_block(room_num_array, check_in, check_out, block_id, discount_percent: 0)
+      @blocks << Hotel::Block.new(room_num_array, check_in, check_out, block_id, discount_percent: 0)
+    end


This method is never called, in your product code or in your tests.

…ons. Logic was a bit too much for me to grasp alone. I Will ask for help. So I decided to finish writing my tests and then will go back and refactor.

…ing towards refactoring and adding more tests for all classes.

… one reservation. Refactor add_reservation_to_block method to loosely couple the Admin and Block classes. Tests write for both refactors.

Some original feedback comments were not addressed for Hotel Revisited. However, the comments I did address deepened my understanding.

…riable @date_range_array. Now we have loosely coupled classes. DateRange#overlap and DateRange#include are up and running.

…ange#date_range_array method and completely removed it.

awilson2017 added 12 commits September 5, 2017 20:45

Create all files for both folders lib and specs

e30668e

Write implimentation and specs for Room class. Write test for paramet…

5e10fe1

…ers passed to Room instances.

Remove Room class and specs because I realized it had no functionalitiy.

3688ea9

Write initialize test for DateRange class and write DateRange implimintation code. Rename Hotel class to Admin class. Stored unused work in unused-work folder for potential future use.

Reservation class: add methods reservation_id and total_cost.

ec3daeb

Made tests for reservation class' added methods. Starting to work on Admin class and tests.

Admin Class: Add methods list_of_rooms, add_reservation, list_reserva…

bd0473f

…tion(date), and list_vacancies. Need to write tests for almost all the listed methods.

realize previous git commits were committed for lib folder.

15c644b

So specs file was not staged. Specs are written for code except list_of_rooms, add_reservation, list_reservation(date), and list_vacancies methods.

Write tests for methods list_of_rooms, add_reservation, and list_rese…

87cc702

…rvations.

Write reserve available room by date range method and tests.

c727184

Big Picture: Refactoring Reservation class to dual as Block and Reser…

07544d9

…vation functions. This commit, refactor the initialize method to take a discount and refactor total_cost method to take discount.

Refactor Admin class to create method create_block_by_date.

11e312e

Admin Class: Add methods create_block_by_date, find_block, list_avail…

a6a88f9

…able_blocked_rooms, and find_rooms_from_block. Also added method add_reservation_to_block.

The tests for refactored Admin class were turned off during wave 3's …

3306518

…block refactor. Work on refactoring the specs for block code.

droberts-sea reviewed Sep 12, 2017

View reviewed changes

awilson2017 added 7 commits October 1, 2017 14:58

try to refactor DateRange class to instructional staff's recommendati…

e8f505b

…ons. Logic was a bit too much for me to grasp alone. I Will ask for help. So I decided to finish writing my tests and then will go back and refactor.

Regarding testing: all tests from the first submission now pass. Work…

727c36e

…ing towards refactoring and adding more tests for all classes.

Add test for calculating both full and discount price

1392f27

Admin Class: Refactor create_block_... method to automatically create…

37f305d

… one reservation. Refactor add_reservation_to_block method to loosely couple the Admin and Block classes. Tests write for both refactors.

complete the desigh_assignemnt.

9f6caa3

Some original feedback comments were not addressed for Hotel Revisited. However, the comments I did address deepened my understanding.

Refactored classes and methods that depended on DateRange instance va…

fbe5f24

…riable @date_range_array. Now we have loosely coupled classes. DateRange#overlap and DateRange#include are up and running.

Completely refactored code so functionality is not dependent on DateR…

980c899

…ange#date_range_array method and completely removed it.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pipes - Angela- Hotel #45

Pipes - Angela- Hotel #45

awilson2017 commented Sep 11, 2017 •

edited by droberts-sea

Loading

droberts-sea commented Sep 12, 2017

droberts-sea Sep 11, 2017

droberts-sea Sep 11, 2017

droberts-sea Sep 11, 2017

droberts-sea Sep 11, 2017

droberts-sea Sep 11, 2017

droberts-sea Sep 11, 2017

droberts-sea Sep 11, 2017

droberts-sea Sep 11, 2017

droberts-sea Sep 11, 2017

droberts-sea Sep 11, 2017

droberts-sea Sep 11, 2017

		@discount_percent = discount_percent


		@reservations_array2 = []

		available_rooms = list_of_rooms

		date_range = DateRange.new(check_in, check_out).date_range_array

Pipes - Angela- Hotel #45

Are you sure you want to change the base?

Pipes - Angela- Hotel #45

Conversation

awilson2017 commented Sep 11, 2017 • edited by droberts-sea Loading

Hotel

Comprehension Questions

droberts-sea commented Sep 12, 2017

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

awilson2017 commented Sep 11, 2017 •

edited by droberts-sea

Loading