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

Amy Lee -- Carets #31

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

Amy Lee -- Carets #31

wants to merge 27 commits into from

Conversation

ayjlee
Copy link

@ayjlee ayjlee commented Sep 11, 2017

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 made Blocks a separate class/distinct object, and I made Block Reservations a subclass of Reservations. Initially, I was just going to make a Blocks class that would somehow relate to or inherit from Reservations. However, I realized a Block was a different kind of container, which dictates which rooms you are allowed to reserve, and what dates. Reservations for rooms in blocks had all the same methods and variables as Reservations, but behaved a bit differently or had different default values. Because these changes were minimal, but significant, I decided to make Block Reservations a subclass of Reservations.
Describe a concept that you gained more clarity on as you worked on this assignment. Over-designing and separately, SRP. I caught myself writing code for features that were not asked for, but that I thought a Hotel reservation system should be capable of. Also, I found that SRP is easy to understand in theory, but hard for me to implement in practice.
Describe a nominal test that you wrote for this assignment. I tested that the available_rooms method was returning an updated array of all available rooms by date as reservations or blocks were made.
Describe an edge case test that you wrote for this assignment. An edge case I tested for in thinking about user input error prevention was raising an error if anybody entered a date from the past for check-in or check-out.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? Writing pseudocode was a good learning experience- it helped me to clarify the design choices I needed to make by encouraging me to narrate the actions I wanted to take and follow the intended consequences of those actions on the classes in my code. However, I think I spent too much time going back and forth with my design, and this meant that I would write some tests, write code, have a thought, write a bunch of code to test the new idea, and then realize that I should write some more tests. Unfortunately, I don't think I implemented TDD practices well (in favor of following the thread of an idea and updated codes as it affected different elements of my program), or the principle of Separation of Concerns. In retrospect, I feel like I MacGyver-ed my way through Hotel- it was fun to think through and create, should work for the given requirements and constraints, but is not the most well-designed or efficient solution.

…older for potential future Reservation class methods inserted
…rough testing required, error handling, and edge case testing required
…o help return a list of all rooms available for a date range; passes a basic test that it will return the correct Boolean value
… an available room for a given date range. passes basic tests. design notes for possible refactoring included
…on object, and changed the order of the initialize arguments
…ndependent class. First attempt at implementing logic for making Block reservations, passing
…oom IDs only, not Room objects, for the purposes of figuring out method logic/figuring out how classes connect and effect each other. all tests passing; will probably change
…be more intuitive with design logic - HotelBooking module has a Hotel, which has Rooms, Reservations, Blocks, and Block Reservations, which in turn have dates
…all_dates variable and refactor/refine everything. Frurther testing needed
…nd blocks, and one array for all reserved dates. Hotel will hav methods to find reservations and blocks by IDs as needed
@tildeee
Copy link

tildeee commented Sep 14, 2017

Hotel

What We're Looking For

Feature Feedback
Baseline
Used git regularly x
Answer comprehension questions x
Design
Each class is responsible for a single piece of the program x
Classes are loosely coupled x
Wave 1
List rooms x
Reserve a room for a given date range x
List reservations for a given date x
Calculate reservation price x
Invalid date range produces an error x
Test coverage x, missing one for "can list rooms" technically
Wave 2
View available rooms for a given date range x
Reserving a room that is not available produces an error x
Test coverage x
Wave 3
Create a block of rooms x
Check if a block has rooms x
Reserve a room from a block x
Test coverage x
Additional Feedback

room.reserve_room(@check_in, @check_out, res_idx, guest_idx)

guest_idx +=1
res_idx +=1
Copy link

Choose a reason for hiding this comment

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

at each iteration, because you're resetting the values of guest_idx and res_idx in lines 164 and 165, incrementing them here doesn't actually affect the code

Your tests pass because you aren't testing for this logic/asserting that the guest and res ids are set!

lib/Booking.rb Outdated
room = HotelBooking::Room.new(i)
standard_rooms << room
i += 1
end
Copy link

Choose a reason for hiding this comment

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

you can probably do this same thing with

NUM_STANDARD_ROOMS.times do |index|
  room = HotelBooking::Room.new(index + 1)
  standard_rooms << room
end

or

(1..NUM_STANDARD_ROOMS).each do |index|
instead of needing to increment with i if you wanted to

@id = id_number
@nightly_rate = nightly_rate
@type = :standard
@reserv_ids = []
Copy link

Choose a reason for hiding this comment

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

What are @reserv_ids for?

@tildeee
Copy link

tildeee commented Sep 23, 2017

Hey Amy! Sorry that my review of your code took a long time.

Overall, I'm really pleased with your Hotel class.

I have questions about how you designed the Room class-- it seems like it has some responsibilities about making block reservations and I'm not sure it's appropriate.

I don't know how much you need Reservation to have id


end

def check_valid_dates(check_in, check_out)
Copy link

Choose a reason for hiding this comment

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

I love all the small helper methods you made :D I wish that there were a better place to put this specific helper method about checking if it's a valid date that wasn't in Hotel, but I can't think of a better place at the moment.

end

if check_out < check_in
raise ArgumentError.new("Invalid Date Range: Check out date is earlier than check-in date.")
Copy link

Choose a reason for hiding this comment

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

You might want to pick a different Error besides ArgumentError (like even writing a custom Error!)

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