Skip to content

Ports- Sav#31

Open
qqdipps wants to merge 85 commits intoAda-C11:masterfrom
qqdipps:master
Open

Ports- Sav#31
qqdipps wants to merge 85 commits intoAda-C11:masterfrom
qqdipps:master

Conversation

@qqdipps
Copy link
Copy Markdown

@qqdipps qqdipps commented Mar 18, 2019

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What was a design challenge that you encountered on this project? I used method calls from other methods and classes and chained them together, I wasn't sure how to avoid this so I tried to minimize dependencies by duck typing, one way object composition, and SRP applied to methods and classes the best I could.
What was a design decision you made that changed over time over the project? Originally I didn't think that room needed its own class. It was a struct in Manifest, but as I progressed I wanted to add behaviors to it so I pulled it out and made it a class on its own.
What was a concept you gained clarity on, or a learning that you'd like to share? I thought that block should inherit from reservation since it seemed like it was like a reservation but with more behaviors. Then I realized that yes they have lots of things in common, but they have a different purpose and that it would be better to create an abstract class Unavailable in which they could both inherit from.
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case? A nominal case tests a normal expectation of how an object should behave. Manifest#list_available_rooms_by_date_range: date_range checked, overlaps reservations(inner check-out/ outer of check-in) returns available rooms.
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case? An edge case tests the outer bounds of possible behaviors of an object. Manifest#list_rooms: returns [] if rooms to list is empty array.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? When I did the pseudocode first I had a better understanding of the helper methods that were required and my code more thoughtful , also testing was easier because I had a more defined idea where I was going.

qqdipps added 30 commits March 11, 2019 14:35
qqdipps added 25 commits March 17, 2019 02:03
… not required for user story nor helper method, same can be accomplisted using def list_available_rooms_by_date_range(date_range:).
…ry wants list of reserved rooms only (seems like to not include block) corresponding tests removed as well.
@CheezItMan
Copy link
Copy Markdown

Hotel

What We're Looking For

Feature Feedback
Baseline
Used git regularly Good number of commits and good commit messages
Answer comprehension questions Check, Glad pseudocode helped!
Design
Each class is responsible for a single piece of the program Check
Classes are loosely coupled Check
Wave 1
List rooms Check
Reserve a room for a given date range Check
List reservations for a given date Check
Calculate reservation price Check
Invalid date range produces an error Check
Wave 2
View available rooms for a given date range Check
Reserving a room that is not available produces an error Check
Wave 3
Create a block of rooms Check
Check if a block has rooms Check
Reserve a room from a block Check
Test coverage 100%
Fundamentals
Names variables, classes and modules appropriately Check
Understanding of variable scope - local vs instance Check
Can create complex logical structures utilizing variables Check
Appropriately uses methods to break down tasks into smaller simpler tasks Check
Understands the differences between class and instance methods Check
Appropriately uses iterators and Enumerable methods Check
Appropriately writes and utilizes classes Check
Appropriately utilizes modules as a namespace Check
Wrap Up
There is a refactors.txt file Check
The file provides a roadmap to future changes Check
Additional Feedback Well done, you hit the learning goals for this project. Good use of inheritance, although the name is a little hard to understand since it really just contains a date range. Nice work!

Comment thread lib/booker.rb
end
block.set_number_available(rooms_collection.length)
rooms_collection.each do |room|
room.unavailable_list << block
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This isn't a problem for this project, but if someone wanted to cancel a block, you'd have to find rooms with the block and delete that from each room. Just a little awkward.

Comment thread lib/booker.rb Outdated
reservation.cost = room.cost_per_night * reservation.duration_in_days * (100 - percent_discount) / 100.0
end

def get_cost_of_booking(reservation:)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This method's a little weird in that you're passing in a reservation and returning the cost of that reservation, which seems that this method is unnecessary.

Comment thread lib/manifest.rb
setup_rooms(rooms_to_set_up)
end

def setup_rooms(rooms_to_set_up)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

these should be private I believe

Comment thread lib/manifest.rb
rooms << RoomWrapper::room(cost: cost_per_night, room_number: rooms.length + 1)
end

def list_rooms(rooms_to_list: rooms)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why is this method simply returning the object passed in as an argument?

Comment thread lib/manifest.rb

def find_unavailable_object(id:)
rooms.each do |room|
room.unavailable_list.each do |unavailable_object|
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unavailable how? A room might be available on a given day and unavailable the next.

Comment thread lib/manifest.rb
end
end

module RoomWrapper
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm uncertain this RoomWrapper module serves much purpose.

Comment thread spec/block_spec.rb
expect(valid_block.number_available).must_equal 0
end

it "will check if room available, that is number_available < 0 will return false" do
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So you don't raise an error if they try to reserve more rooms from a block than are in the block?

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