branches - vi by criacuervos · Pull Request #27 · Ada-C12/hotel

criacuervos · 2019-09-09T05:41:21Z

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
What was a design challenge that you encountered on this project?	when i got to wave 3, i had trouble overall grasping the concept of a block. once that became clear, i didn't struggle as much with realizing how all the different parts could come together.
What was a design decision you made that changed over time over the project?	i didn't really have any major design changes- i just changed some aspects about where i instantiated objects in different classes
What was a concept you gained clarity on, or a learning that you'd like to share?	this project helped me gain clarity on writing tests, and how the overall process of tdd is very helpful. writing my tests first allowed me to feel confident about what it is i was looking to return in a method that i wanted to write.
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case?	a nominal test i wrote was just checking if a class was initiated. that just checks that the object exists, and does what its supposed to.
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case?	an edge case i had was in my tests for date range, if the user tries to create a 0 length stay. normally people staying in a hotel will book at least one night, but in an instance where there might be user error in input, the program can be prepared to handle it.
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	pseudocode was very important to me- i used wipeboards, pen and paper, and hashed out lines in my code constantly! it was crucial to me in this project and i was pseudocoding every single step of the way

…itialization and checking to see that length is < 0" "

… passed all

…ude? method in date_range class & test

…passed

…th updated tests that pass. currently working on overlap tests, some of which are breaking...

…ction and tests

…ange

tildeee · 2019-09-16T05:04:54Z

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)	no tests to check for edge cases like if the date ranges overlap, but the implementation to raise the error is there
List reservations for a given date	yes, but the test isn't quite all the way there
Calculate reservation price	yes
Invalid date range produces an error	yes
Test coverage	tests nominal cases
Wave 2
View available rooms for a given date range	yes
Reserving a room that is not available produces an error	yes, but no test
Test coverage	missing thoroughness is `ReservationMaker`, but covered well in `DateRange`
Wave 3
Create a block of rooms	yes
Check if a block has rooms	your implementation assumes that a block has as many available rooms as can exist (a block doesn't have a limited number of rooms available)
Reserve a room from a block	yes-- tests live in `Block` tests
Test coverage	mostly, yes!

Code Review

Baseline	Feedback
Used git regularly	yes
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	yes
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	Leaned on `.each`-- Not a bad thing, though it may be interesting to see if there are chances for refactoring
Appropriately writes and utilizes classes	yes
Appropriately utilizes modules as a namespace	yes
Wrap Up
There is a refactors.txt file	nope!
The file provides a roadmap to future changes	nope!

Overall 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!

Overall, your code is very neat, logical, and flows well together! I feel that your code is nice, straightforward, and concise. I truly feel confident in your solid grasp on programming fundamentals based on what I see in your project submission.

I think that your choices in how you implemented the DateRange class and utilized it is particularly strong! You have strong, clear tests there, and really good code as the implementation.

Overall, your tests are extremely well-organized and have all of the correct parts and syntax (with great before blocks, and all the right sections of Arrange-Act-Assert). However, with a lot of the tests, I see that you Assert some really good fundamental bits, but you miss checking some critical pieces of functionality. For example, adding a reservation to @reservations in ReservationMaker should change the length of the @reservations array, but that detail isn't checked. I would love to see your test-writing skills be pushed to always ask, "what are MORE details that I can check, that would make sure that this method I'm testing is working in every angle I can think of?" You often test that the return value is the right data-type, but miss confirming other details.

I have a couple of other suggestions for how the tests can be improved-- they're minor and actionable and left as comments on your code, so let me know if you have questions on those!

Overall, I think that your project looks GREAT! I think there are a feeew bugs, and I think that your tests can improve by becoming more detailed. However, as I mentioned before, I think that your project shows that you can navigate and write your way around a larger project-- well done

tildeee · 2019-09-16T05:06:00Z

lib/date_range.rb

+    def date_overlap?(other) 
+      return (check_in...check_out).cover?(other.check_in) ||
+      (other.check_in...other.check_out).cover?(check_in) || 
+      (other.check_in...other.check_out) == (check_in...check_out)


Clever and concise!

tildeee · 2019-09-16T05:06:07Z

lib/reservation.rb

+  class Reservation
+    attr_reader :check_in, :check_out, :room
+    attr_accessor :date_range
+    def initialize(date_range, room = nil, block_reference = nil )


I like your optional arguments a lot! :) :) :) Keep this up!

tildeee · 2019-09-16T05:06:21Z

lib/reservation_maker.rb

+      date_range = Hotel::Date_Range.new(check_in, check_out)
+      @blocks.each do |block|
+        if !block.date_range.include?(check_in) 
+          raise ReservationError, "Date range is not included in existing block!"


Hm, this code reads to me like "Check every reservation. If the date_range made above with our proposed check_in and check_out overlaps with any reservation's check_in and check_out, then we should raise a ReservationError." Does this mean that an error will be raised if Room No. 1 has an overlapping reservation, even if Rooms 2-20 are available? In either case, this section of the code doesn't have a unit test, so it's hard for me to check at the moment.

tildeee · 2019-09-16T05:06:40Z

test/block_test.rb

+
+  describe "add block to list of blocks" do
+    it "returns a list of all blocks" do
+      block_array = @reservation_maker.add_block(@block)


Because this Act step is about a method that exists in the ReservationMaker class (aka @reservation_maker.add_block), I would probably move this test to the ReservationMaker test!

tildeee · 2019-09-16T05:07:07Z

test/block_test.rb

+
+  describe "reserve in block" do
+    it "reserves a room inside of a block" do 
+      @block_reservation = @reservation_maker.reserve_from_block(@check_in, @check_out)


Same as above: Because this Act step is about a method that exists in the ReservationMaker class (aka @reservation_maker.reserve_from_block), I would probably move this test to the ReservationMaker test!

tildeee · 2019-09-16T05:07:17Z

test/block_test.rb

+      @block_reservation = @reservation_maker.reserve_from_block(@check_in, @check_out)
+      expect(@block_reservation).must_be_kind_of Hotel::Reservation 
+    end
+    it "raises an exception for a date range not in a block" do


Nice edge case!

tildeee · 2019-09-16T05:08:26Z

test/block_test.rb

+    it "raises an exception for a date range not in a block" do
+      @reservation_maker.add_block(@block)
+
+      other_check_in = Date.new(2019-8-01)


Watch out -- Ruby interprets this technically as "Make a new Date with the value of 2019-8-1, aka Jan 01, 2010"!

tildeee · 2019-09-16T05:09:19Z

test/reservation_maker_test.rb

+      it "is a list of reservations" do
+        check_in = @date
+        check_out = @date + 4
+        reservation = @reservation_maker.reserve_room(check_in, check_out)


To test this add_reservation method in isolation, you probably don't need to call reserve_room as part of the Arrange step, you probably can just make a new instance of Reservation! Let me know if that feels confusing

tildeee · 2019-09-16T05:09:42Z

test/reservation_maker_test.rb

+
+        expect(reservation_list).must_be_kind_of Array 
+        reservation_list.each do |reservation|
+          reservation.must_be_kind_of Reservation 


it's unclear from this test, but reservation_list is going to be an empty array here! To set this test up properly, it probably would be best if we made sure that @reservation_maker had some reservations made as part of the Arrange before we check the result of reservations_lookup

tildeee · 2019-09-16T05:09:56Z

test/reservation_maker_test.rb

+        date_range = (check_in...check_out)
+
+        room_list = @reservation_maker.available_rooms(date_range)
+        expect(room_list).must_be_kind_of Array


Hm, I wish this test checked the length of room_list too!

criacuervos added 12 commits September 3, 2019 14:12

created the classes and instantiated reservation object

3f8312a

Initialized date_range class, as well as added tests to verify its in…

7a1face

…itialization and checking to see that length is < 0" "

added to the date range, reservation, and reservation_maker tests and…

9107567

… passed all

added simplecov to the mix!

5f46a08

Added reservations_lookup for given date code & test, as well as incl…

a9b6862

…ude? method in date_range class & test

added tests and now on wave 2

6b01fa9

created date overlap method and first test about equivalent date ranges

3dbcb01

created an overlap? method in date_range class, along with tests and …

848af5f

…passed

updated reserve_room method and added available rooms method along wi…

0e071ab

…th updated tests that pass. currently working on overlap tests, some of which are breaking...

fixed overlap test issues- all tests now passing

d35c8fb

updated block class, added tests, and added reserve room in block fun…

a44dbc1

…ction and tests

added test to raise error if reserving room in block for wrong date r…

af89d34

…ange

tildeee reviewed Sep 16, 2019

View reviewed changes

criacuervos added 2 commits September 25, 2019 17:05

finished prompt questions

f8f7adc

finished break week homework assignment

41cfd5b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

branches - vi#27

branches - vi#27
criacuervos wants to merge 14 commits intoAda-C12:masterfrom
criacuervos:master

criacuervos commented Sep 9, 2019

Uh oh!

tildeee commented Sep 16, 2019

Uh oh!

tildeee Sep 16, 2019

Uh oh!

tildeee Sep 16, 2019

Uh oh!

tildeee Sep 16, 2019

Uh oh!

tildeee Sep 16, 2019

Uh oh!

tildeee Sep 16, 2019

Uh oh!

tildeee Sep 16, 2019

Uh oh!

tildeee Sep 16, 2019

Uh oh!

tildeee Sep 16, 2019

Uh oh!

tildeee Sep 16, 2019

Uh oh!

tildeee Sep 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

criacuervos commented Sep 9, 2019

Hotel

Comprehension Questions

Uh oh!

tildeee commented Sep 16, 2019

Hotel

What We're Looking For

Test Inspection

Code Review

Overall Feedback

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