Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Isabel Suchanek -- Carets #25

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

isabeldepapel
Copy link

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? I was thinking of making the Block class a subclass of Reservation since they had similar attributes, but they were different enough structurally (a block could correspond to many reservations) that I couldn't figure out how to make it worked. I was thinking of using a mixin since both classes were "reservable" and I originally had this set up to calculate total cost for both classes, and also see if they included a particular date (in the block or reservation). But when I changed my Reservation class to include @Rate as an attribute (which allowed me just call total_cost from Reservation), I didn't need it in the mixin anymore, so the mixin is now more about error checking than anything else. I'm not sure this is the most logically sorted out system.
Describe a concept that you gained more clarity on as you worked on this assignment. Working with class variables. I initially was working on this without class variables, but it was a little clunky and I was curious as to how it would work if I used them. Some of the code is more streamlined now, but the testing can get kind of weird--I realized if I didn't reset the class variables every time I tested their contents, they would not return what I expected.
Describe a nominal test that you wrote for this assignment. When testing the Block class to reserve a room from within that class, I called the method on a room in the block to see if it had been reserved, and that the check-in/out times on the reservation matched the check-in/out times on the block.
Describe an edge case test that you wrote for this assignment. Re. testing reserving a room from the Block class, I also checked that the code would raise an error if I tried to reserve a room that wasn't in the block, and I also tested against double bookings by reserving the room in the block, then checking to make sure that it would throw an error if I tried to reserve the room again, whether from within the block or outside it.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? Pretty good w/the pseudocode. But sometimes I get too excited about the pseudocode if I figure out how to solve something and I start writing the code before fleshing out the tests.

@CheezItMan
Copy link

Hotel

What We're Looking For

Feature Feedback
Baseline
Used git regularly Good number of commit and good commit messages.
Answer comprehension questions Check, The class variables you used are a good use-case for the before block in testing. The side-effects are also a reason we use class variables sparingly.
Design
Each class is responsible for a single piece of the program Pretty good job here. You did a good job of separating out the roles.
Classes are loosely coupled There is some tighter coupling here. Room and Hotel are pretty tightly coupled. The user of the App would need to use the Room class to do things like reserve a room or block.
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
Invalid date range produces an error Check through mix-in
Wave 2
View available rooms for a given date range
Reserving a room that is not available produces an error
Wave 3
Create a block of rooms Very DRYly done. Nice
Check if a block has rooms Check
Reserve a room from a block Check
Test coverage 97%
Additional Feedback Nice work incorporating mix-in into the project.

Copy link

@CheezItMan CheezItMan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few observations.

end

def <=>(other_room)
room_num <=> other_room.room_num

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

lib/room.rb Outdated

private

def array_include_date?(array, start_date, end_date)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is a little too generic, what is the array parameter supposed to be?

It is however very DRY.

return room.reserve(check_in, check_out, discounted_rate)
end

def self.all

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good following of the existing pattern, it makes it easier to add a CSV or database later.

lib/hotel.rb Outdated

end

def reserve(start_date, end_date, room)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an observation, a user would need to identify a room object first before trying to reserve it. Why not just have them provide a room number and have this method first find the room.

That way the user doesn't have to know about Room objects or the Room class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants