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 - Sara S - Hotel #32

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

Conversation

CheerOnMars
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 had to decide how to group methods into blocks. I would like to create a Block Class in order to move towards classes with fewer responsibility, but I decided to wait until the refactor portion so I could make sure I could get the methods to work first.
Describe a concept that you gained more clarity on as you worked on this assignment.
Describe a nominal test that you wrote for this assignment. For the test for the calculating reservation cost method, I made sure that a reservation of 5 nights would cost $1000.
Describe an edge case test that you wrote for this assignment. One edge case was putting in a reservation end date that was before the reservation start date.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I thought it was helpful to write the pseudocode first because it helped me keep the classes, variables, and methods straight. It's easy to lose track, but the pseudocode was a useful reference sheet.

…rooms and views a list of rooms that aren't reserved for a given range
…d access_reservation method to remove strftime(%Y-%m-%d) functionality
def add_reservation(reservation)
#note: this is tightly coupled, I think.
@reservations << reservation
@reservation = reservation

Choose a reason for hiding this comment

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

Why set this as an instance variable here?

end

def calc_reservations(reservation)
reservation.price_night * reservation.calc_duration

Choose a reason for hiding this comment

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

I think this logic would be better inside the reservation object


def create_block(input)

if input[:block_start_date] == nil || input[:block_end_date] == nil

Choose a reason for hiding this comment

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

All of this data validation should be in an initialize somewhere

@kariabancroft
Copy link

Hotel

What We're Looking For

Feature Feedback
Design
Demonstrated classes having a single responsibility Room for improvement - it is easy to fill up the HotelAdmin class with lots of logic that doesn't need to be there. I think you have a few instances of this, so keep this in mind.
Demonstrated loose coupling Yes - though the HotelAdmin is doing some things that it doesn't need to do, I think you generally do try to keep the separation between objects.
Methods demonstrate a good use of encapsulation, inputs and outputs Yes
Wave 1 requirements Yes
Wave 2 requirements Yes
Wave 3 requirements Yes

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