Skip to content

Amanda Ungco Hotel -Attempt #1#48

Open
unkcodesquick wants to merge 41 commits intoAda-C10:masterfrom
unkcodesquick:master
Open

Amanda Ungco Hotel -Attempt #1#48
unkcodesquick wants to merge 41 commits intoAda-C10:masterfrom
unkcodesquick:master

Conversation

@unkcodesquick
Copy link

@unkcodesquick unkcodesquick commented Sep 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? I wanted to put an instance of room in the reservation objects but ultimately took it out because when testing my code, I got stuck in an infinite loop of my reservation referencing a room which was referencing a reservation which was referencing a room. I decided the object of room was unnecesarry to the reservation class and instead added just the room ID to the reservation.
Describe a concept that you gained more clarity on as you worked on this assignment. Class methods, the use of self, how classes interact.
Describe a nominal test that you wrote for this assignment. Expecting that any of my methods that asked for a list was returning an array and that the first element in the array was an instance of a class.
Describe an edge case test that you wrote for this assignment. I did not get to write very many edge cases which i'm sure I will find when i continue to work with my code, but one edge case was dealing with overlapping dates, in my tests to reserve a room and return available rooms, I accidentally created a bunch of overlapping reservations which was great to build an accidental edge case.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I like the method, I think the most difficult part is deciding on and diving into a design. As my code stands now, it only works with the tests I've created, I want to add a lot more edge cases to make sure the program can handle much more.

@dHelmgren
Copy link

Hotel

What We're Looking For

Feature Feedback
Baseline
Used git regularly Yeah!
Answer comprehension questions Yep!
Design
Each class is responsible for a single piece of the program Yeah!
Classes are loosely coupled So, your reservation_manager ends up pretty tightly coupled with your other classes. You can mitigate this by using Factory methods to set up your rooms, and by using duck typing to keep calls outside of reservation_manager from being stuck in lock step with the other classes.
Wave 1
List rooms ReservationManager#rooms
Reserve a room for a given date range ReserMgr#reserve_room
List reservations for a given date ReserMgr#booked_reservations
Calculate reservation price Reservation#total_cost
Invalid date range produces an error Yep!
Test coverage Needs more edge cases!
Wave 2
View available rooms for a given date range ReserMgr#available_rooms
Reserving a room that is not available produces an error It doesn't appear to be possible given your setup, but it seems you don't create an error for the case where there aren't enough rooms.
Test coverage Needs more edge cases
Wave 3
Create a block of rooms create_block
Check if a block has rooms No
Reserve a room from a block WIP code
Test coverage Tests are somewhat fleshed out, mostly complete.
Fundamentals
Names variables, classes and modules appropriately Yea!
Understanding of variable scope - local vs instance Yes!
Can create complex logical structures utilizing variables Yes!
Appropriately uses methods to break down tasks into smaller simpler tasks Yeah, but you could do this even more!
Understands the differences between class and instance methods Yes
Appropriately uses iterators and enumerables Hella!
Appropriately writes and utilizes classes Yes!
Appropriately utilizes modules as a mechanism to encapsulate functionality Yees!
Wrap Up
There is a refactors.txt file There is!
The file provides a roadmap to future changes It's brief, but there are road mapped changes there. Were there any more you were considering?
Additional Feedback So, you are missing some tests that would improve your code, and you didn't get far into wave 3, which effects the outcome here. What would a jerk do to intentionally break your code? Test THAT.

@blocks = []
@rooms = []
(1..number_of_rooms).each do |number|
@rooms << Room.new(number)

Choose a reason for hiding this comment

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

This should be a factory method in Room.




def initialize(number, number_of_rooms:1, check_in:, check_out:, discount_rate: 1)

Choose a reason for hiding this comment

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

number is not a great name for this parameter. I'd opt for something like id_num

def reserve_room(check_in, check_out, number_of_rooms: 1 )
new_reservation = Reservation.new(@reservations.length + 1, check_in: check_in, check_out: check_out)
assigned_rooms = available_rooms(check_in, check_out).last(number_of_rooms)
assigned_rooms.each do |room|

Choose a reason for hiding this comment

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

What happens if there aren't enough rooms?

assigned_rooms = available_rooms(check_in, check_out).last(number_of_rooms)
assigned_rooms.each do |room|
new_reservation.rooms << room.id
find_room(room.id).reservations << new_reservation

Choose a reason for hiding this comment

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

in lieu of doing these here, I would give Room and Reservation a method to make the assignment. It's a small but important piece of making your code more loosely coupled.

new_reservation = Reservation.new(@reservations.length + 1, check_in: check_in, check_out: check_out)
assigned_rooms = available_rooms(check_in, check_out).last(number_of_rooms)
assigned_rooms.each do |room|
new_reservation.rooms << room.id

Choose a reason for hiding this comment

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

Does this work? I only see an attr_reader?

expect(@hotel_ada.booked_reservations(@test_date)).must_be_kind_of Array
#binding.pry
expect(@hotel_ada.booked_reservations(@test_date)[0]).must_be_instance_of Hotel::Reservation
expect(@hotel_ada.booked_reservations(@test_date).length).must_equal 3

Choose a reason for hiding this comment

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

What does your program do if it isn't handed a proper date?

#binding.pry
expect(@hotel_ada.available_rooms('2018-08-20', '2018-08-23').length).must_equal 16
expect(@hotel_ada.available_rooms('2018-08-20', '2018-08-23')[0]).must_be_kind_of Hotel::Room
#expect(@hotel_ada.available_rooms("08.23.2018", "08.25.2018")[0].id).must_equal #rooom ID

Choose a reason for hiding this comment

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

You should be testing to see that the code can handle different kinds of reservation date collisions here, not merely that you are getting the right data types!

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