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 - Abinnet #41

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

Ampers - Abinnet #41

wants to merge 29 commits into from

Conversation

Abiaina
Copy link

@Abiaina Abiaina 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? Deciding to make a room class or just an array of room ids. Talking it over with Chris during our on-on-one and in class when I recognized that the room class is just holding very little info.
Describe a concept that you gained more clarity on as you worked on this assignment. I improved on design, I used more flow charts/ mapping to come up with a basic design, then wrote the tests and continuously ran the tests as I wrote the code itself.
Describe a nominal test that you wrote for this assignment. testing if a reservation is able to be made when dates are given and there is available rooms
Describe an edge case test that you wrote for this assignment. Testing if a reservation is able to be made when dates are given but there are no more available rooms. Trying to book more than 20 rooms in a single date range.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I actually liked it, the pseudo code part on whiteboards was really helpful in determining edge cases and what variables/parameters I would use for specific methods.

Abiaina added 25 commits March 5, 2018 14:33
…ost instance variable in reservation an admin spec.
…ation instances array returns booked room id
…ects with a specifc date, tests pass for intersect and no date intersects
…ion to existing reservations prevents double booking
… to new room for reservation attempts to rooms that are already booked
@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, your Admin class is doing a bit much, but overall pretty good.
Demonstrated loose coupling Very good work here, Admin depends on Reservation and Block, and Block depends on Reservation
Methods demonstrate a good use of encapsulation, inputs and outputs Pretty good, see my comments.
Wave 1 requirements Check
Wave 2 requirements Check, but you have a bug in your logic, see my notes.
Wave 3 requirements Check
Summary Nice work, you showed a lot of growth here in design. Overall nicely done. You have some bugs in your code, but the design is pretty good overall. Checkout our reference implementation

raise ArgumentError.new("#{room_id} is booked for these dates")
end

if available_rooms.count == 0

Choose a reason for hiding this comment

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

Ummm.. if there were no rooms then available_rooms would not include room_id.

lib/admin.rb Outdated
@reservations.each do |reservation|
valid_check_in = check_in.between?(reservation.start_date, (reservation.end_date - 1))

valid_check_out = check_out.between?((reservation.start_date + 1), reservation.end_date)

Choose a reason for hiding this comment

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

What about if the reservation occurs entirely between check_in and check_out?

lib/admin.rb Outdated
@reservations << Reservation.new(check_in, check_out, room_id)
end

def available_rooms(check_in, check_out)

Choose a reason for hiding this comment

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

To have wave 3 complete you may also need to check the blocks, or make sure add_reservation considers blocks.

@@ -0,0 +1,47 @@
require 'date'

class Reservation

Choose a reason for hiding this comment

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

I would suggest a method in reservation which either takes a Reservation or a date range and returns true if it overlaps with this reservation and false if it does not. It might make the logic easier in Admin.

@bookings[0].room_id.must_equal 1
end

it "calculates the length of stay" do

Choose a reason for hiding this comment

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

This is really a test of the Reservation class.

end

it "add reservation adds reservation to reservations list" do
before_rezs = @blockroom_test.reservations.dup

Choose a reason for hiding this comment

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

I'm unsure why you need to duplicate the array.

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