hotel by jazziesf · Pull Request #23 · Ada-C10/hotel

jazziesf · 2018-09-10T07:36:14Z

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?	The structor of my program came from a what I felt was a similar project ride share. My designed started to unfold as I made the block reservation methods. Which lead to me think it may be better to have a block reservation class rather than tie all reservations together. I am still thinking about my design decisions.
Describe a concept that you gained more clarity on as you worked on this assignment.	It was the possible need for a block reservation class and the possibility removing my room class.
Describe a nominal test that you wrote for this assignment.	A nominal test did the my method create 20 rooms in the hotel
Describe an edge case test that you wrote for this assignment.	If a reservation exceeded the block limit.
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	I really enjoy writing pseudocode first but felt I may have jumped in to early with this project before I had a solid design.

dHelmgren · 2018-09-24T04:50:36Z

Hotel

What We're Looking For

Feature	Feedback
Baseline
Used git regularly	You should commit more. Try adding and committing when things are stable rather than when they are working.
Answer comprehension questions	Good answers, though for number 2 I'm more curious what sorts of high level programming concepts you understood better
Design
Each class is responsible for a single piece of the program	Not really. You have `FrontDesk` checking the internals of your `Reservation` class frequently. See inline comments for `search_reserved_by_date`.
Classes are loosely coupled	No. They would be much less closely coupled if not for the above problem.
Wave 1
List rooms	Yep
Reserve a room for a given date range	Yep
List reservations for a given date	Yep
Calculate reservation price	Yep
Invalid date range produces an error	No, see inline!
Test coverage	You've commented out a key test for this wave, and your program fails it.
Wave 2
View available rooms for a given date range	Yep
Reserving a room that is not available produces an error	Yep!
Test coverage	This wave needed much more extensive testing, see inline!
Wave 3	You have an implementation that seems like you misunderstood the directions, see my comment in "raises error if standard reservation conflicts with block reservation".
Create a block of rooms	Yep
Check if a block has rooms	I don't see anywhere that explicitly does this.
Reserve a room from a block	Yep!
Test coverage	Given your assumptions about the directions, these tests are good, but you should be testing more and weirder cases.
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	I think you could do a better job of this. Especially when you have a situation where you're accessing the guts of another class, best practice is to pass the values you want to compare against to the class.
Understands the differences between class and instance methods	Yes, but I don't see any class methods.
Appropriately uses iterators and enumerables	Yes! Love the use of Reject
Appropriately writes and utilizes classes	You could do this better too. `Room` is so small as to hardly need to be a class, and `Reservation` needs to take charge of its internal comparisons.
Appropriately utilizes modules as a mechanism to encapsulate functionality	No, you don't use a module to wrap the Hotel.
Wrap Up
There is a refactors.txt file	Yes!
The file provides a roadmap to future changes	These are a bit vague, but I agree with all of them.
Additional Feedback	You are doing a lot of smart stuff here. I like your approach to checking if a reservation conflicts a lot (test the positive case, throw a ! on the whole thing). It seems to me from reading the tests and what is and isn't covered that you built your code first and your tests second, which helps explain why some tests are missing or failing. TTD is a really valuable methodology to follow, and I think you'd benefit from knowing what part of your code you are building first. I think you might benefit from taking a second look at how we understand and construct classes according to POODR over the break. Your use of classes and methods are okay but could greatly improve with better application of theory.

dHelmgren · 2018-09-24T03:46:09Z

lib/front_desk.rb

+      rooms << Room.new({
+        :room_number => room_number
+      })
+    end


I think this block makes more sense as a factory method inside Room.

dHelmgren · 2018-09-24T03:47:33Z

lib/front_desk.rb

+    input = {}
+    input[:room_number] = room_number
+    input[:start_date] = start_date
+    input[:end_date] = end_date


Cool use of a hash to keep your code loose! Just as an FYI, keyword parameters do the same thing but keep greater clarity as to what's actually getting passed in!

lib/front_desk.rb

dHelmgren · 2018-09-24T03:56:57Z

lib/front_desk.rb

+    results = []
+
+    @reservations.each do |reservation|
+      if reservation.start_date <= search_date && search_date < reservation.end_date


So, this expression in the if should probably be the return value of an instance method for the Reservation class.

lib/front_desk.rb

dHelmgren · 2018-09-24T04:26:51Z

spec/front_desk_spec.rb

+
+    it "raises error if standard reservation conflicts with block reservation" do
+    @admin.block_hold(('2018-02-01'),('2018-02-11'),2, "Martinez")
+    expect {@admin.reserve_room(1,('2018-02-01'),('2018-02-10'))}.must_raise StandardError


I think I see what happened with your code: We expect that people can still reserve rooms inside a block. By this, I mean that if I Devin create a block of four rooms, I don't pay for them all at once. I am telling the hotel "I think I can help you fill these rooms because of my event". Each person in my party still has to make their own reservation after the block is created.

dHelmgren · 2018-09-24T04:29:03Z

spec/front_desk_spec.rb

+    # it "raises an error if start date after the end date" do
+    #   room = @admin.reserve_room(6,('2018-02-05'),('2018-02-03'))
+    #   expect{room}.must_raise StandardError
+    # end


Why is this test commented out? It's a super important test!

dHelmgren · 2018-09-24T04:32:28Z

lib/front_desk.rb

+  end
+
+  def reserve_room(room_number,start_date, end_date)
+


You should be checking here to see if the start_date comes before the end date! Not having this fundamentally breaks a lot of the other code you've written!

dHelmgren · 2018-09-24T04:38:34Z

spec/front_desk_spec.rb

+      expect(@admin.search_reserved_by_date('2018-02-05').length).must_equal 3
+    end
+
+    it "returns zero for day of" do


I'd specify "date of checkout" in the title here.

dHelmgren · 2018-09-24T04:40:21Z

spec/front_desk_spec.rb

+
+    it "allows a reservation to end on the same day another reservation starts" do
+      expect(@admin.reserve_room(5,('2018-02-01'),('2018-02-03'))).must_be_kind_of Reservation
+    end


You are missing important test cases for overlapping reservations! What if the reservation surrounds an existing reservation? What if it's completely inside another reservation?

I'm fairly certain that your code would pass the tests, but you don't ensure that confidence by doing exhaustive testing!

jazziesf added 8 commits September 5, 2018 19:32

Wave 1 in progress

96eb3b1

Wave 1 Complete

c451828

Wave 2 Test in progress

0440fe7

Wave 2 complete

4a88b12

Test in progess Wave 3

8f06c10

Block Reservation Complete

4ab3f83

Add Search method to Block Reservation

f7fe641

refactors.txt created

614b471

dHelmgren reviewed Sep 24, 2018

View reviewed changes

jazziesf added 2 commits September 29, 2018 17:56

Add design-activity.md

7f4a459

Made adjustments recommended

1c7889d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

hotel#23

hotel#23
jazziesf wants to merge 10 commits intoAda-C10:masterfrom
jazziesf:master

jazziesf commented Sep 10, 2018

Uh oh!

dHelmgren commented Sep 24, 2018

Uh oh!

dHelmgren Sep 24, 2018

Uh oh!

dHelmgren Sep 24, 2018

Uh oh!

Uh oh!

dHelmgren Sep 24, 2018

Uh oh!

Uh oh!

dHelmgren Sep 24, 2018

Uh oh!

dHelmgren Sep 24, 2018

Uh oh!

dHelmgren Sep 24, 2018

Uh oh!

dHelmgren Sep 24, 2018

Uh oh!

dHelmgren Sep 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

jazziesf commented Sep 10, 2018

Hotel

Comprehension Questions

Uh oh!

dHelmgren commented Sep 24, 2018

Hotel

What We're Looking For

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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