TIME - Angela by angethuy · Pull Request #33 · Ada-C13/hotel

angethuy · 2020-03-09T15:50:23Z

Assignment Submission: Hotel

Congratulations! You're submitting your assignment. Please reflect on the assignment with these questions.

Reflection

Question	Answer
What was a design challenge that you encountered on this project?	I spent a lot of time considering making blocks a child class of Reservation but I didn't think the relationship warranted managing inheritance when the difference between a Reservation and a Block was just 1) more rooms and guests and 2) an optional discount rate.
What was a design decision you made that changed over time over the project?	I chose to make "occupancy" a special variable - it tracks rooms belonging to a reservation (one to [1-5] relationship) as an array of hashes with room id, guest name. This was a pain to manage. I'm not sure I would dig myself into the same rabbit hole in the future.
What was a concept you gained clarity on, or a learning that you'd like to share?	Overlapping dates are much simpler than they seem!!! If we can trust that a date range implies contiguous dates, then to test overlap, we need to check if 1) a proposed start date falls inside of the range EXCLUDING end date, or or 2) the proposed end date falls inside of the range EXCLUDING start date. We only need to check for two cases.
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case?	Testing the creation of a new Reservation is a nominal test because it directly ties to a user story.
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case?	I had an edge case to verify that an ArgumentError is raised if I try to make a reservation where the end date occurs before the start date. It's an edge case because it's a wild and whacky situation that should not occur but just in case it does...
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	I didn't spend enough time writing pseudocode and will likely dedicate more time to that step for my next project. I'm still having trouble writing tests before my code but my process for writing code and tests occurs more in tandem now compared to my habits from before. Using guard and coverage have helped a lot.

…ing.

kaidamasaki · 2020-03-13T20:38:38Z

Hotel

Section 1: Major Learning Goals

Criteria	yes/no, and optionally any details/lines of code to reference
Practices SRP by having at least two separate classes with distinct responsibilities, and test files for these two classes	✔️
Overall, demonstrates understanding instance variables vs. local variables. (There aren't unnecessarily too many instance variables, when it should be a local variable)	✔️
For each test file, tests demonstrate an understanding of instantiating objects properly, and using Arrange-Act-Assert	✔️
Practices pseudocode and TDD, and reflected on it by filling out the reflection questions	Somewhat and is making good progress.
Practices git with at least 15 small commits and meaningful commit messages	Please commit more. I promise it will be helpful later. :)

Section 2: Code Review and Testing Requirements

Criteria	yes/no, and optionally any details/lines of code to reference
There is a class that represents a reservation, and a second class that holds/manages a collection of reservations through composition (instance variable)	✔️
The logic for checking if a reservation's date overlaps with another reservation's date is complex logic that is separated into method(s) (and potentially class(es))	✔️
The logic for checking if a reservation's date overlaps with another reservation's date has unit tests	No tests on `overlaps?`.
All of the given tests run and pass	✔️
A test coverage tool is installed and used, and shows 95% test coverage	✔️ 🎉 100% 🎉

Section 3: Feature Requirements

Feature Requirement: There is a method that...	yes/no
gives back a list of rooms, and it's tested	✔️
creates a specific reservation for a room for a given date range, and it has nominal test cases	✔️
creates a specific reservation for a room for a given date range, and it tests an edge case, such as no available room, or invalid date range	No edge case test.
gives back a list of reservations on a given date. Its tests include a test that makes several reservations for a given date	✔️
calculates the total price for a reservation	✔️
gives back a list of available rooms for a given date range, and it has nominal test cases	✔️
gives back a list of available rooms for a given date range, and it has edge test cases, such as no available room, or invalid date range	No edge case test.
creates a block of rooms	✔️
reserves a room from a block	✔️

Overall Feedback

Overall Feedback	Criteria	yes/no
Green (Meets/Exceeds Standards)	14+ total in all sections	✔️
Yellow (Approaches Standards)	9-13 total in all sections
Red (Not at Standard)	0-8 total in all sections, or assignment is breaking/doesn’t run with less than 5 minutes of debugging

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

I am particularly impressed by the way that you organized your methods within your classes and made good use of Ruby's built-in methods.

I do see some room for improvement around testing edge cases.

Code Style Bonus Awards

Was the code particularly impressive in code style for any of these reasons (or more...?)

Quality	Yes?
Perfect Indentation
Elegant/Clever	✅
Descriptive/Readable	✅
Concise	✅
Logical/Organized	✅

kaidamasaki

Great job! Here are a few little things that you can do to clean up your code even more. 😄

kaidamasaki · 2020-03-13T18:00:05Z

.gitignore

+
+#system
+
+.DS_store


You can ignore .DS_Store everywhere by creating a global gitignore file.

https://help.github.com/en/github/using-git/ignoring-files#configuring-ignored-files-for-all-repositories-on-your-computer

kaidamasaki · 2020-03-13T18:02:14Z

lib/date_range.rb

+    attr_reader :check_in, :check_out, :nights_spent
+
+    def initialize(check_in, check_out)
+      raise ArgumentError, 'Invalid date range given.' unless check_out > check_in


It's helpful to include the invalid arguments when you raise an ArgumentError.

Suggested change

raise ArgumentError, 'Invalid date range given.' unless check_out > check_in

raise ArgumentError, "Invalid date range given. check_in: #{check_in} check_out: #{check_out}" unless check_out > check_in

kaidamasaki · 2020-03-13T18:05:30Z

lib/date_range.rb

+      raise ArgumentError, 'Invalid date range given.' unless check_out > check_in
+      @check_in = check_in
+      @check_out = check_out
+      @nights_spent = (@check_out - @check_in) 


Parenthesis here are unnecessary:

Suggested change

@nights_spent = (@check_out - @check_in)

@nights_spent = @check_out - @check_in

kaidamasaki · 2020-03-13T18:05:49Z

lib/date_range.rb

+    end
+
+    def to_id
+      return (@check_in.year.to_s + @check_in.mon.to_s + @check_in.mday.to_s)


You don't need parenthesis around return values:

Suggested change

return (@check_in.year.to_s + @check_in.mon.to_s + @check_in.mday.to_s)

return @check_in.year.to_s + @check_in.mon.to_s + @check_in.mday.to_s

kaidamasaki · 2020-03-13T18:07:00Z

lib/date_range.rb

+    end
+
+    def overlap? (other_check_in, other_check_out)
+      return ((@check_in..@check_out).cover? (other_check_in)) || ((@check_in-1...@check_out).cover? (other_check_out))


Clever use of cover?!

kaidamasaki · 2020-03-13T19:23:19Z

lib/reservation_manager.rb

+    # Parameters: 
+    # -type: symbol (:SINGLE, :BLOCK) that indicates type of reservation
+    # -range: DateRange object 
+    # -occupancy: Hash or String
+    # Returns: newly created Reservation object


I really like that you documented your parameters but it would be more helpful if you mentioned more than just the type but also what they represent.

Really the only confusing bit here is occupancy. Everything else is pretty self explanatory or well explained (like type).

kaidamasaki · 2020-03-13T19:26:12Z

lib/reservation_manager.rb

+    # Parameters: Date object 
+    # Returns: an Array of all Reservation instances that contain that date
+    def find_reservations_by_date(check_in, check_out)
+      by_date = @reservations.select { |reservation| reservation.dates.overlap?(check_in, check_out)}


This works because of implicit return, but I don't think you meant to assign here (since you don't use the variable):

Suggested change

by_date = @reservations.select { |reservation| reservation.dates.overlap?(check_in, check_out)}

return @reservations.select { |reservation| reservation.dates.overlap?(check_in, check_out)}

kaidamasaki · 2020-03-13T19:26:37Z

lib/reservation_manager.rb

+    end
+
+    def find_reservations_by_room(room)
+      by_room = @reservations.select { |reservation| reservation.occupancy.detect { |occupancy| occupancy[:room] == room }}


See above.

Suggested change

by_room = @reservations.select { |reservation| reservation.occupancy.detect { |occupancy| occupancy[:room] == room }}

return @reservations.select { |reservation| reservation.occupancy.detect { |occupancy| occupancy[:room] == room }}

kaidamasaki · 2020-03-13T19:31:53Z

test/reservation_manager_test.rb

+    it "populates valid room objects" do
+      expect(@reservation_manager.rooms[1]).must_be_kind_of Hotel::Room
+    end


A better check here would be to loop through all of the rooms with individual assertions:

Suggested change

it "populates valid room objects" do

expect(@reservation_manager.rooms[1]).must_be_kind_of Hotel::Room

end

it "populates valid room objects" do

@reservation_manager.rooms.each do |room|

expect(room).must_be_kind_of Hotel::Room

end

end

kaidamasaki · 2020-03-13T19:34:07Z

test/reservation_manager_test.rb

+  describe "create reservation" do
+    before do
+      @room = Hotel::Room.new(100)
+      @occupancy = [{:room => @room, :guest => "Picchu"}]


Is this like the pokemon?

angethuy added 9 commits March 2, 2020 15:39

initial scaffolding + tests passing

d507ad3

basic class structure complete, all tests passing

cd99558

Added .collide? method to test for date overlaps

22af1d2

rooms no longer track reservations

b70128d

reservation objects now accept multiple rooms (blocks). includes test…

af700ad

…ing.

reservation_manager now finds reservations by date. includes tests.

1b95126

spacing cleanup and additional commenting

7ec02e2

added date range validation

1f4b4fc

added ability to find available rooms given a date range

4b12b60

angethuy changed the title ~~Angela - TIME~~ TIME - Angela Mar 9, 2020

kaidamasaki reviewed Mar 13, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TIME - Angela#33

TIME - Angela#33
angethuy wants to merge 9 commits intoAda-C13:masterfrom
angethuy:master

angethuy commented Mar 9, 2020

Uh oh!

kaidamasaki commented Mar 13, 2020

Uh oh!

kaidamasaki left a comment

Uh oh!

kaidamasaki Mar 13, 2020

Uh oh!

kaidamasaki Mar 13, 2020

Uh oh!

kaidamasaki Mar 13, 2020

Uh oh!

kaidamasaki Mar 13, 2020

Uh oh!

kaidamasaki Mar 13, 2020

Uh oh!

kaidamasaki Mar 13, 2020

Uh oh!

kaidamasaki Mar 13, 2020

Uh oh!

kaidamasaki Mar 13, 2020

Uh oh!

kaidamasaki Mar 13, 2020

Uh oh!

kaidamasaki Mar 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	raise ArgumentError, 'Invalid date range given.' unless check_out > check_in
	raise ArgumentError, "Invalid date range given. check_in: #{check_in} check_out: #{check_out}" unless check_out > check_in

	@nights_spent = (@check_out - @check_in)
	@nights_spent = @check_out - @check_in

	return (@check_in.year.to_s + @check_in.mon.to_s + @check_in.mday.to_s)
	return @check_in.year.to_s + @check_in.mon.to_s + @check_in.mday.to_s

	by_date = @reservations.select { \|reservation\| reservation.dates.overlap?(check_in, check_out)}
	return @reservations.select { \|reservation\| reservation.dates.overlap?(check_in, check_out)}

	by_room = @reservations.select { \|reservation\| reservation.occupancy.detect { \|occupancy\| occupancy[:room] == room }}
	return @reservations.select { \|reservation\| reservation.occupancy.detect { \|occupancy\| occupancy[:room] == room }}


		#system

		.DS_store

Conversation

angethuy commented Mar 9, 2020

Assignment Submission: Hotel

Reflection

Uh oh!

kaidamasaki commented Mar 13, 2020

Hotel

Section 1: Major Learning Goals

Section 2: Code Review and Testing Requirements

Section 3: Feature Requirements

Overall Feedback

Additional Feedback

Code Style Bonus Awards

Uh oh!

kaidamasaki left a comment

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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants