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

Ampers: Alex #24

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

Ampers: Alex #24

wants to merge 19 commits into from

Conversation

brownav
Copy link

@brownav brownav commented Mar 12, 2018

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? On whether to include a room class or not -- I decided the class would be too scant to be of much use, and instead kept rooms as an attribute of Hotel::Admin.
Describe a concept that you gained more clarity on as you worked on this assignment. Testing before writing methods to help gain clarity on an approach before coding, pseudocoding, and being more robust with testing edge-cases and parameters.
Describe a nominal test that you wrote for this assignment. Testing if errors were raised for out-of-range or constraint-breaking inputs where used.
Describe an edge case test that you wrote for this assignment. Testing if errors were raised for nil or missing inputs.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I improved on this aspect as the project went further along, as the methods became more complicated to construct, I took more effort to write pseudocode first, then construct tests, then fill in the pseudocode with actual code. This approach actually proved really helpful despite taking some deliberate getting used to at first.

@CheezItMan
Copy link

CheezItMan commented Mar 19, 2018

Hotel

What We're Looking For

Feature Feedback
Design
Demonstrated classes having a single responsibility For the most part yes, More work should be handed off to reservation. Also you are using a hash for a block. That could very well be a class in it's own right.
Demonstrated loose coupling Check
Methods demonstrate a good use of encapsulation, inputs and outputs Some issues here, see my notes.
Wave 1 requirements You've got some issues with your logic, specifically you're removing rooms permanently if they are ever reserved, even for one day.
Wave 2 requirements See my notes.
Wave 3 requirements You seem to be confusing a block with a reservation. Instead of holding rooms for future reservations you're taking the Block as a bulk reservation.
Summary There's some clever work here specifically with how you see if a reservation overlaps with a date, but there's also a lot of problematic code. Checkout our reference implementation

USA
@@ -0,0 +1 @@
WHERE IS ADA?

Choose a reason for hiding this comment

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

Here!

1215 4th Ave
Suite 1050
Seattle, WA 98161

lib/admin.rb Outdated
@blocks = []

@reservations.each do |reservation|
overlap = reservation.dates & days

Choose a reason for hiding this comment

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

This won't work unless days has an exact date of a reservation. So this requires your reservations to keep all the dates it's reserved.

However doing it this way does make & work, clever.

You also will need to consider rooms in blocks however.


admin.book_room(res1)

admin.rooms.length.must_equal free_rooms - 1

Choose a reason for hiding this comment

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

This test doesn't make sense, the number of rooms doesn't change when a room is booked, the number of free rooms in a date range would change.

@reservations.each do |reservation|
overlap = reservation.dates & days
if overlap.length > 0
@rooms.pop

Choose a reason for hiding this comment

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

Removing the room from the list doesn't make sense, since when I book a room the hotel doesn't lose a room permanently.

lib/admin.rb Outdated
book_room(reservation)
end

def create_block_of_rooms(reservation, num_rooms)

Choose a reason for hiding this comment

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

What about:

  1. Creating a block with a discounted room rate.
  2. Is a block a Reservation?
  3. Is a room really reserved, if it's in the block, or is it just being held for later?

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