Ports - Stephanie by smarchante1 · Pull Request #33 · Ada-C11/hotel

smarchante1 · 2019-03-18T08:14:28Z

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question	Answer
What was a design challenge that you encountered on this project?	One major challenge I had was deciding if I wanted to keep a date range class that I originally had. I ended up removing it and think that might not have been the best decision.
What was a design decision you made that changed over time over the project?	Initially I wanted to store block reservations in my main Reservation Manager class. I changed this to storing block reservations in a hotel block class.
What was a concept you gained clarity on, or a learning that you'd like to share?	I have a better understanding of how classes can interact with one another through the methods that are created in each class. This also made the project quite challenging.
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 making sure a reservation was added to the list of reservations.
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 wrote was checking to make sure a check out date wasn't before a check_in date.
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	I really enjoyed this workflow. It helped me to get a better understanding of the methods I needed, and then slowly work towards writing them.

…ng repository and all commits are gone :(

CheezItMan · 2019-03-24T22:44:55Z

Hotel

What We're Looking For

Feature	Feedback
Baseline
Used git regularly	Good number of commits and good commit messages
Answer comprehension questions	Check
Design
Each class is responsible for a single piece of the program	For what's been done yes.
Classes are loosely coupled	Check
Wave 1
List rooms	Check
Reserve a room for a given date range	Check
List reservations for a given date	Check
Calculate reservation price	Check, but see my inline notes, the if statement is... odd
Invalid date range produces an error	Check
Test coverage	88.44% coverage :(
Wave 2
View available rooms for a given date range	Missing
Reserving a room that is not available produces an error	Missing
Wave 3
Create a block of rooms	Missing
Check if a block has rooms	Missing
Reserve a room from a block	Missing
Fundamentals
Names variables, classes and modules appropriately	See my inline notes for some of your method names.
Understanding of variable scope - local vs instance	Check
Can create complex logical structures utilizing variables	Check
Appropriately uses methods to break down tasks into smaller simpler tasks	Check, for what was done
Understands the differences between class and instance methods	Only instance methods here.
Appropriately uses iterators and `Enumerable` methods	Very few examples of Enumerable methods here.
Appropriately writes and utilizes classes	Check
Appropriately utilizes modules as a namespace	Check
Wrap Up
There is a refactors.txt file	Check
The file provides a roadmap to future changes	It's starting to
Additional Feedback	As we noted in a 1-on-1, this was a rough project for you. Not a great outcome, but you had the bones of a great project here. You had good classes and a good rough outline of your design. Unfortunately it wasn't implemented and that's unfortunate. Take a look at my feedback and ask me questions if something is unclear. This is a tough industry, but you do have the talent to be very successful here. Don't let what other people say distract you. You can make a great impact here! If you are feeling down and upset please talk to me, Sarah or Dee and let us support you!

CheezItMan · 2019-03-24T22:14:43Z

+      expect(@reservations.length).must_equal(1)    
+    end
+
+    it "raises an error if the room is not available" do


Your room here is still available so the ArgumentError is not being raised. This is a good start on testing the double-book issue.

CheezItMan · 2019-03-24T22:22:45Z

+      # unless room_availability(date_range.check_in, date_range.check_out).include?(room)
+      #   raise ArgumentError, "The room you've selected is not available for these dates."
+      # end
+      reservation = Hotel::Reservation.new(room, date_range)


No check that the room is already reserved.

CheezItMan · 2019-03-24T22:24:13Z

+    def initialize(check_in, check_out)
+      @check_in = check_in
+
+      if check_out < check_in


You're checking this twice, here and with the total_stay method. A little odd. Also this if statement doesn't check to see if the dates are equal, but your total_stay method did.

CheezItMan · 2019-03-24T22:26:46Z

+      return total_nights
+    end
+
+    def date_check(date)


This method is a little vaguely named. I suggest naming it within_range to tell if a date is within the DateRange.

CheezItMan · 2019-03-24T22:28:01Z

+      return valid_date
+    end
+
+    def daterange_check(check_in, check_out)


This method is also a little misleading, I would call it overlap and pass another DateRange instance to compare. If this is the method you're using and you don't expect users to use date_check directly, I would also make that method private.

CheezItMan · 2019-03-24T22:32:11Z

+    def initialize
+      @reservations = []
+      @block_reservations = []
+    end


I suggest also keeping rooms as an array of numbers and setting it with your hotel_rooms method, so you don't have to repeatably call that method.

CheezItMan · 2019-03-24T22:35:14Z

+
+  describe 'date check method' do
+    it 'returns true if checked date is within daterange (false if not)' do
+      expect(@test_daterange.date_check(Date.new(2019,3,12))).must_equal(true)


You should also check the start_date and end_date and check to see date_check on those dates.

CheezItMan · 2019-03-24T22:36:03Z

+
+  describe 'daterange check method' do
+    it 'returns true if checked date is within daterange (false if not)' do
+      expect(@test_daterange.daterange_check(Date.new(2019,3,12),Date.new(2019,3,14))).must_equal(true)


Similar to the above, check the edge cases, date ranges that end on the start date of your date_range and another that starts on the end_date.

Then try some overlaps.

CheezItMan · 2019-03-24T22:37:09Z

+      [*1..20].each do |room|
+        @frontdesk.book_reservation(room, Hotel::DateRange.new(Date.new(2019,3,15),Date.new(2019,3,18)))
+      end
+      expect(@frontdesk.room_availability(Date.new(2019,3,15), Date.new(2019,3,18))).must_equal([])


CheezItMan · 2019-03-24T22:38:35Z

+      expect(@frontdesk.res_by_date(Date.new(2002,3,15)) ).must_be_instance_of(Array)
+    end
+
+    it "returns array of all booked reservations" do


You should also test that it has the given reservations.

smarchante1 · 2019-03-24T22:56:31Z

Hi Chris! I think I might have misunderstood the Hotel Revisted project. I submitted my project when it was due and thought that we were supposed to make changes even though we didn’t receive feedback, to start the break week refactor project. I commented out the original submission for my project and I was working on the refactor with date ranges today under the impression that it was break week homework. If you look at the original commit, that was my original submission going into wave 3. Is it possible for you to take a look at it?

…

On Sun, Mar 24, 2019 at 3:45 PM Chris M ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In spec/reservation_manager_spec.rb <#33 (comment)>: > + end + + describe "book reservation method" do + before do + @Reservations = [] + @test_range = Hotel::DateRange.new(Date.new(2019,3,11), Date.new(2019,3,15)) + end + + it "stores booked reservations in reservations array" do + res1 = @frontdesk.book_reservation(1, @test_range) + @Reservations << res1 + + ***@***.***).must_equal(1) + end + + it "raises an error if the room is not available" do Your room here is still available so the ArgumentError is not being raised. This is a good start on testing the double-book issue. ------------------------------ In lib/reservation_manager.rb <#33 (comment)>: > + @Reservations = [] + @block_reservations = [] + end + + def hotel_rooms + rooms = [*1..20] + return rooms + end + + # basic reservation methods + def book_reservation(room, date_range) + # raising the error but test says nothing was raised + # unless room_availability(date_range.check_in, date_range.check_out).include?(room) + # raise ArgumentError, "The room you've selected is not available for these dates." + # end + reservation = Hotel::Reservation.new(room, date_range) No check that the room is already reserved. ------------------------------ In lib/date_range.rb <#33 (comment)>: > @@ -0,0 +1,38 @@ +require 'date' + +module Hotel + class DateRange + attr_reader :check_in, :check_out + + def initialize(check_in, check_out) + @check_in = check_in + + if check_out < check_in You're checking this twice, here and with the total_stay method. A little odd. Also this if statement doesn't check to see if the dates are equal, but your total_stay method did. ------------------------------ In lib/date_range.rb <#33 (comment)>: > + else + @check_out = check_out + end + + if total_stay <= 0 + raise ArgumentError, "Invalid date range provided." + end + + end + + def total_stay + total_nights = (check_out - check_in).to_i + return total_nights + end + + def date_check(date) This method is a little vaguely named. I suggest naming it within_range to tell if a date is within the DateRange. ------------------------------ In lib/date_range.rb <#33 (comment)>: > + raise ArgumentError, "Invalid date range provided." + end + + end + + def total_stay + total_nights = (check_out - check_in).to_i + return total_nights + end + + def date_check(date) + valid_date = (date >= @check_in && date < @check_out) + return valid_date + end + + def daterange_check(check_in, check_out) This method is also a little misleading, I would call it overlap and pass another DateRange instance to compare. If this is the method you're using and you don't expect users to use date_check directly, I would also make that method private. ------------------------------ In lib/hotel_block.rb <#33 (comment)>: > @@ -0,0 +1,64 @@ +module Hotel + class HotelBlock + attr_reader :room, :check_in, :check_out, :cost, :reservations, :hotel Looks like you got started here, but didn't get far. Just one observation, I think the hotel instance method is unneeded. You should have something to keep track of the rooms in the block however. ------------------------------ In lib/reservation.rb <#33 (comment)>: > @@ -0,0 +1,23 @@ +require 'date' + +module Hotel + class Reservation + attr_reader :room, :date_range, :cost + + def initialize(room, date_range, cost: 200) + @room = room + @date_range = date_range + @cost = cost + + end + + def total_cost + if cost == 200 Why this if statement here? Why not just use the value of cost? ------------------------------ In lib/reservation_manager.rb <#33 (comment)>: > @@ -0,0 +1,74 @@ +require 'date' + +module Hotel + class ReservationManager + attr_reader :reservations, :block_reservations + + def initialize + @Reservations = [] + @block_reservations = [] + end I suggest also keeping rooms as an array of numbers and setting it with your hotel_rooms method, so you don't have to repeatably call that method. ------------------------------ In spec/date_range_spec.rb <#33 (comment)>: > + end + + it "raises an Argument Error if the check in date is the same as the check out date" do + expect{Hotel::DateRange.new(Date.new(2019,3,11), Date.new(2019,3,11))}.must_raise ArgumentError + end + end + + describe 'total stay method' do + it 'calculates the total stay' do + ***@***.***_daterange.total_stay).must_equal(4) + end + end + + describe 'date check method' do + it 'returns true if checked date is within daterange (false if not)' do + ***@***.***_daterange.date_check(Date.new(2019,3,12))).must_equal(true) You should also check the start_date and end_date and check to see date_check on those dates. ------------------------------ In spec/date_range_spec.rb <#33 (comment)>: > + ***@***.***_daterange.total_stay).must_equal(4) + end + end + + describe 'date check method' do + it 'returns true if checked date is within daterange (false if not)' do + ***@***.***_daterange.date_check(Date.new(2019,3,12))).must_equal(true) + ***@***.***_daterange.date_check(Date.new(2019,3,14))).must_equal(true) + ***@***.***_daterange.date_check(Date.new(2019,3,20))).must_equal(false) + ***@***.***_daterange.date_check(Date.new(2019,3,15))).must_equal(false) + end + end + + describe 'daterange check method' do + it 'returns true if checked date is within daterange (false if not)' do + ***@***.***_daterange.daterange_check(Date.new(2019,3,12),Date.new(2019,3,14))).must_equal(true) Similar to the above, check the edge cases, date ranges that end on the start date of your date_range and another that starts on the end_date. Then try some overlaps. ------------------------------ In spec/reservation_manager_spec.rb <#33 (comment)>: > + + end + + describe 'room availability method' do + it "returns available rooms" do + [*1..5].each do |room| + @frontdesk.book_reservation(room, Hotel::DateRange.new(Date.new(2019,3,15),Date.new(2019,3,18))) + end + ***@***.***_availability(Date.new(2019,3,15), Date.new(2019,3,18))).must_equal([*6..20]) + end + + it "returns empty array if all rooms are booked" do + [*1..20].each do |room| + @frontdesk.book_reservation(room, Hotel::DateRange.new(Date.new(2019,3,15),Date.new(2019,3,18))) + end + ***@***.***_availability(Date.new(2019,3,15), Date.new(2019,3,18))).must_equal([]) 👍 ------------------------------ In spec/reservation_manager_spec.rb <#33 (comment)>: > + describe "search reservation by date method" do + before do + @frontdesk.book_reservation(1, + Hotel::DateRange.new(Date.new(2019,3,5), Date.new(2019,3,9))) + @frontdesk.book_reservation(1, + Hotel::DateRange.new(Date.new(2019,4,11), Date.new(2019,4,12))) + @frontdesk.book_reservation(5, + Hotel::DateRange.new(Date.new(2019,3,5), Date.new(2019,3,8))) + @res_list = @frontdesk.res_by_date(Date.new(2019,3,7)) + end + + it 'returns an empty array if there are no reservations during selected date' do + ***@***.***_by_date(Date.new(2002,3,15)) ).must_be_instance_of(Array) + end + + it "returns array of all booked reservations" do You should also test that it has the given reservations. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#33 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Ad3zmYVX3swJmv2leCbw2_VhE33ZOdRHks5vZ__vgaJpZM4b5Ljf> .

…sign activity

smarchante1 added 11 commits March 18, 2019 01:04

finished refactoring code. couldn't make pull request. Had to reforki…

73d737e

…ng repository and all commits are gone :(

adding commit history screencaps

42da91f

adding daterange class back into design

c53f88b

added tests for date range class

a88a1e3

refactored book reservation method

1b96196

updated reservation spec file

8038561

added date check method and associated tests

e5119ae

readded original date range check method

bc8b870

refactoring room availability method and tests

76d44a5

finished refactoring book reservation method and tests

5dc0bea

refactored reservations by date method and accompanying tests

ddc30a6

CheezItMan reviewed Mar 24, 2019

View reviewed changes

smarchante1 added 7 commits March 24, 2019 18:13

Adding book block reservation method and associated tests

9bb7e8c

fixed typo with total cost method

50e572c

fixed issue with book reservation method test

0f6c2e0

add rooms available and block date check methods and associated tests

aec5bd5

adding design activity file for Hotel Refactor

cb3a255

added tests for block methods and refactored failing tests. Added de…

adffad9

…sign activity

debugging tests

0b1e77a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ports - Stephanie#33

Ports - Stephanie#33
smarchante1 wants to merge 18 commits intoAda-C11:masterfrom
smarchante1:master

smarchante1 commented Mar 18, 2019

Uh oh!

CheezItMan commented Mar 24, 2019

Uh oh!

CheezItMan Mar 24, 2019

Uh oh!

CheezItMan Mar 24, 2019

Uh oh!

CheezItMan Mar 24, 2019

Uh oh!

CheezItMan Mar 24, 2019

Uh oh!

CheezItMan Mar 24, 2019

Uh oh!

CheezItMan Mar 24, 2019

Uh oh!

CheezItMan Mar 24, 2019

Uh oh!

CheezItMan Mar 24, 2019

Uh oh!

CheezItMan Mar 24, 2019

Uh oh!

CheezItMan Mar 24, 2019

Uh oh!

smarchante1 commented Mar 24, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

smarchante1 commented Mar 18, 2019

Hotel

Comprehension Questions

Uh oh!

CheezItMan commented Mar 24, 2019

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!

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!

smarchante1 commented Mar 24, 2019 via email

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants