Leaves - Morgan #48

morganschuler · 2019-09-11T05:22:02Z

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
What was a design challenge that you encountered on this project?	A design project I encountered on this project was on how to structure my classes. I found myself creating more than ended up being necessary, so throughout the project I had to restructure my classes to be more efficient.
What was a design decision you made that changed over time over the project?	I decided to remove my room class after starting because I found that I could obtain my reservations just as easily.
What was a concept you gained clarity on, or a learning that you'd like to share?	I gained concept on was object composition, and understand the difference between this and inheritance.
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case?	A nominal case, or a case that provided expected behavior, that I wrote for this project was testing a regular date range when submitting a reservation.
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case?	An edge case that I tested for in this project was testing to see if a date was outside of an accepted block. This was an edge case because it did not provide input that was expected to pass, but rather wanted to test in the event of code that was expected to fail
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	I feel that I should have used pseudocode more in order to better test my code. I found that I wanted to start coding immediately, but when I went back and planned out my program with pseudocode I was able to better understand how my program would progress.

beccaelenzil · 2019-09-12T15:37:18Z

Hotel

What We're Looking For

Test Inspection

Workflow	yes / yes but no test / no
Wave 1
List rooms	yes
Reserve a room for a given date range	yes
Reserve a room (edge case)
List reservations for a given date	I don't see a test for this.
Calculate reservation price	yes
Invalid date range produces an error	yes
Test coverage	In order to get the tests to run I had to comment out `require_relative ../lib/room` from your test_helper. Make sure you run your tests before turning in to make sure everything's good to go. Tests produce 2 Failures and 7 Errors.
Wave 2
View available rooms for a given date range	yes -- this functionality is in 'available_room_by_date_range'
Reserving a room that is not available produces an error	I think I see this functionality, but how is `selected_room` not an array and a single room. Where is the room chosen?
Test coverage
Wave 3
Create a block of rooms
Check if a block has rooms
Reserve a room from a block
Test coverage	date_range and reservation, 100% -- hotelController 86%

Code Review

Baseline	Feedback
Used git regularly	I assume their are more commits in the original pull request. Let me talk to another instructor about how to do this fix better next time using all the wonder of git.
Answer comprehension questions	yes
Design
Each class is responsible for a single piece of the program	yes
Classes are loosely coupled	yes
Fundamentals
Names variables, classes and modules appropriately	yes
Understanding of variable scope - local vs instance	The tests are giving you lots of warnings, `warning: instance variable @room not initialize.` It seems like you may have more instance variables than you need, or perhaps this is just an artifact of having a room class and then removing it. Additionally, you need to make sure you have writers for your instance variables if needed.
Can create complex logical structures utilizing variables	yes
Appropriately uses methods to break down tasks into smaller simpler tasks	yes
Understands the differences between class and instance methods	yes
Appropriately uses iterators and `Enumerable` methods	yes
Appropriately writes and utilizes classes	yes
Appropriately utilizes modules as a namespace	yes
Wrap Up
There is a refactors.txt file	yes
The file provides a roadmap to future changes	Add a bit more so that you remember where the trouble was.

Overall Feedback

You're off to a great start! 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!

You've made good decisions about class structure and encapsulation of different functionality in different methods.

You still need to work on putting it all together. You could start with some of the failing tests to clean up the functionality. SEe inline comments for suggestions on what's happening with each of your tests. You also need a few more tests to check for both positive and negative outcomes (when you're allowed to make a reservation and when you're not).

beccaelenzil · 2019-09-12T15:02:30Z

test/test_helper.rb

+require_relative "../lib/hotel_controller"
+require_relative "../lib/reservation"
+require_relative "../lib/date_range"
+require_relative "../lib/room"


This last require_relative "../lib/room" should have been removed. I had to comment it out to make the tests run.

beccaelenzil · 2019-09-12T15:05:04Z

test/hotel_controller_test.rb

+      expect(new_range.given_range).must_be Hash
+    end
+
+    it "takes available rooms from unavailable hash and creates available array" do


Did you decide not to do this? If so, remove the test.

beccaelenzil · 2019-09-12T15:07:39Z

test/reservation_test.rb

+      @end_date = "2020-08-04"
+      @start_date = "2020-08-06"
+
+      new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil)


This test produces an error because this line appropriately raises an error:

Suggested change

new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil)

new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil)

This line needs to be in the expect{} so that the test doesn't itself raise an error.

Suggested change

new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil)

expect{new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil)}.must_raise StandardError

beccaelenzil · 2019-09-12T15:12:32Z

test/reservation_test.rb

+        selected_room = @room
+        new_reservation = Hotel::Reservation.new(@start_date, @end_date, selected_room)
+
+        new_reservation.nights = (@end_date) - (@start_date)


Nights doesn't have a writer method so you can not perform this assignment. This is evidenced by the test error

Hotel::Reservation::nights /Users/becca/Documents/GitHub/c12-student-repos/hotel/test/reservation_test.rb:59: warning: instance variable @room not initialized test_0002_calculates how many nights the guest is paying for ERROR (0.00s) NoMethodError: NoMethodError: undefined method `nights=' for #<Hotel::Reservation:0x00007fca6c887d48> Did you mean? nights

beccaelenzil · 2019-09-12T15:18:06Z

test/hotel_controller_test.rb

+      date_range = Hotel::HotelController.new
+      given_range = Hotel::DateRange.new(start_date, end_date)
+
+      expect(date_range.given_range).must_include date


Were you trying to provide date_range with given_range as an attribute? Your constructor for HotelController does not have anyway to assign as date_range:

def initialize # @reservation @date_range @rooms = [] @reservations = []

test_0004_puts unavailable rooms into a hash ERROR (0.00s) NoMethodError: NoMethodError: undefined method `given_range' for #<Hotel::HotelController:0x00007fca6c89d5a8> /Users/becca/Documents/GitHub/c12-student-repos/hotel/test/hotel_controller_test.rb:167:in `block (3 levels) in <top (required)>'

beccaelenzil · 2019-09-12T15:19:52Z

test/hotel_controller_test.rb

+      end_date = Date.parse("2016/8/12")
+      selected_room = @room
+
+      new_reservation = Hotel::Reservation.new(start_date_new, end_date_new, selected_room)


This is a good test, but you have not defined start_date_new and end_date_new.

test_0003_ensures there is no overlap with dates ERROR (0.00s) NameError: NameError: undefined local variable or method `start_date_new' for #<#<Class:0x00007fca6b1d0eb0>:0x00007fca6a931cf0> Did you mean? start_date /Users/becca/Documents/GitHub/c12-student-repos/hotel/test/hotel_controller_test.rb:155:in `block (3 levels) in <top (required)>'

beccaelenzil · 2019-09-12T15:20:32Z

test/reservation_test.rb

+
+        # start_date = Date.parse("2016/8/4")
+        # end_date = Date.parse("2016/8/6")
+        # selected_room = @room


Why is this code commented?

test_0002_checks the date is an instance of Date class FAIL (0.00s) Expected nil to be an instance of Date, not NilClass. /Users/becca/Documents/GitHub/c12-student-repos/hotel/test/reservation_test.rb:99:in `block (3 levels) in <top (required)>'

beccaelenzil · 2019-09-12T15:23:04Z

test/date_range_test.rb

+      date = Date.parse("2016/8/7")
+      new_range = Hotel::DateRange.new(start_date, end_date)
+
+      expect(new_range).must_include date


I believe must_include will only check if a value is in an array.

test_0001_determines whether a date is in a given range FAIL (0.00s) Expected #<Hotel::DateRange:0x00007fca6c039c40 @start_date=#<Date: 2016-08-04 ((2457605j,0s,0n),+0s,2299161j)>, @end_date=#<Date: 2016-08-06 ((2457607j,0s,0n),+0s,2299161j)>> (Hotel::DateRange) to respond to #include?. /Users/becca/Documents/GitHub/c12-student-repos/hotel/test/date_range_test.rb:23:in `block (3 levels) in <top (required)>'

beccaelenzil · 2019-09-12T15:35:07Z

lib/reservation.rb

+    # end 
+
+    def in_date_range?(date)
+      return @date_range.in_date_range?(date)


Do you realize you have a recursive call here?

morganschuler added 2 commits September 10, 2019 22:11

Added files from old hotel project

894b9ac

updated test files

0d3902d

beccaelenzil reviewed Sep 12, 2019

View reviewed changes

morganschuler added 23 commits September 23, 2019 11:19

Updated initialize and module in reservation class

7dac5a0

updated method in date_range class to check if date falls within range

05a501f

Updated methods in date_Range to check for overlap

e57e5a8

Updated logic in method to find available room in hotel controller

7bc35b5

Removed unnecessary methods from reservation

f6c758c

restructured tests for DateRange

6af6dd1

Updated date_range tests

cabda3a

restarted tests for DateRange and all passing now with corrections

cc3d185

Added design-activity file

53ee627

Deleted room class and worked on tests

af315be

Answered questions for design activity

80c440f

Updated questions about my code

d26047f

restarted tests for hotel controller based on changes

8c13a11

removed room from test helper

9f8e512

new tests for hotel controller

b71658d

updated method in hotel controller

0f21031

updated tests for hotel controller

f6683f6

updated methods in hotel controller

90fe3e3

Updated design activity page

ff49f85

Updated methods for reservation

08ce15f

Add more edge cases to date range tests

16c6f93

updated tests with correct syntax for error

719d57e

updated refactors file

ac1757a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Leaves - Morgan #48

Leaves - Morgan #48

morganschuler commented Sep 11, 2019

beccaelenzil commented Sep 12, 2019

beccaelenzil Sep 12, 2019

beccaelenzil Sep 12, 2019

beccaelenzil Sep 12, 2019

beccaelenzil Sep 12, 2019

beccaelenzil Sep 12, 2019

beccaelenzil Sep 12, 2019

beccaelenzil Sep 12, 2019

beccaelenzil Sep 12, 2019

beccaelenzil Sep 12, 2019

	new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil)
	new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil)

	new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil)
	expect{new_reservation = Hotel::Reservation.new(@start_date, @end_date, nil)}.must_raise StandardError

Leaves - Morgan #48

Are you sure you want to change the base?

Leaves - Morgan #48

Conversation

morganschuler commented Sep 11, 2019

Hotel

Comprehension Questions

beccaelenzil commented Sep 12, 2019

Hotel

What We're Looking For

Test Inspection

Code Review

Overall Feedback

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