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

Ports - Jessica #27

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

Ports - Jessica #27

wants to merge 57 commits into from

Conversation

jessicacaley
Copy link

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What was a design challenge that you encountered on this project? One design challenge I encountered was trying to reduce dependencies while still maintaining single responsibility. At certain crossroads, it felt like keeping dependencies down meant adding more than one responsibility to one class.
What was a design decision you made that changed over time over the project? I feel like my structure from the beginning remained relatively solid, but when I tried to add in functionality for blocks, I originally attempted to just do that within HotelManager. I quickly realized that, for my specific needs and solution, I needed to create a Block class to store information.
What was a concept you gained clarity on, or a learning that you'd like to share? I gained a lot of clarity on testing, especially the red/green/refactor concept. Because sometimes I didn't know what I necessarily wanted to test for at first, I especially became familiar with the idea of writing the code before the test but then, after writing the test, going back to try to break it by removing the relevant code (my own personal green/red/green/refactor?)
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case? An example of a nominal test I wrote for this assignment is checking that the program can reserve a specific room from a hotel block. It's a nominal case because it's something I expect the program to do on a typical run, if everything's going according to plan.
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case? Using the block example, some edge case tests I wrote were that if the method was fed a collection of 5 rooms, it would create the block just fine, but if it was fed a collection of 6 rooms, it would raise an argument error. This is an edge case because it's just on the border of what I expect the program to have to do/deal with. A customer should be able to book 5 but not 6 rooms for a block.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I don't feel like I've gotten the hang of pseudocode yet... (I know it's highly individual, but maybe watching someone work through pseudocode could be helpful). I don't know if it counts as pseduocode, but there was a point when I definitely had to write out physically in words, "when ____, I want ___ to happen. When _____, I want ____ to happen but I don't want _____ to happen." That helped. As far as tests then code, I felt decent about it towards the end. I started off on a good foot, but by Wave 2 or 3 I tried launching in a writing a bunch of code at once before any tests, and when it didn't work, I had no easy way of knowing why. I ended up deleting a lot of my work and then going back through with a more piece-by-piece test/write code process that made my work cleaner and it became easier to build upon solid foundations.

…g tests, refactor and clean, simplify arguments for blocks
@kaidamasaki
Copy link

kaidamasaki commented Mar 25, 2019

Hotel

What We're Looking For

Feature Feedback
Baseline
Used git regularly Yes.
Answer comprehension questions Yes.
Design
Each class is responsible for a single piece of the program Mostly. FrontDesk is responsible for a larger than expected share of the work though.
Classes are loosely coupled Yes.
Wave 1
List rooms Yes.
Reserve a room for a given date range Yes.
List reservations for a given date Yes.
Calculate reservation price Yes.
Invalid date range produces an error Yes.
Test coverage Yes.
Wave 2
View available rooms for a given date range Yes.
Reserving a room that is not available produces an error Yes.
Test coverage Yes.
Wave 3
Create a block of rooms Yes.
Check if a block has rooms Yes.
Reserve a room from a block Yes.
Test coverage Yes.
Fundamentals
Names variables, classes and modules appropriately Yes.
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 Yes.
Understands the differences between class and instance methods N/A (design didn't use class methods)
Appropriately uses iterators and Enumerable methods Yes.
Appropriately writes and utilizes classes Mostly. Most of your classes were a little on the lean side.
Appropriately utilizes modules as a namespace Yes.
Wrap Up
There is a refactors.txt file Yes
The file provides a roadmap to future changes Yes
Additional Feedback Good job! It looks like you had trouble leveraging Object Oriented programming, aside from FrontDesk most of your classes didn't have much functionality in them. However, your variables were clearly named, your code was rather clean and you finished all three waves!

@@ -0,0 +1,2 @@
class AvailabilityError < StandardError

Choose a reason for hiding this comment

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

Custom error class! 🎉

raise ArgumentError, "Block doesn't exist" unless block
end

def validate_size(number_of_rooms:)

Choose a reason for hiding this comment

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

I would consider moving this code into Block since the FrontDesk doesn't really need to know about the maximum number of rooms a block can have.

end

def validate_size(number_of_rooms:)
max_rooms_for_block = 5

Choose a reason for hiding this comment

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

This should be a constant, probably also in Block. The reason a constant would be useful here is if we change the rules about how many rooms can be used in a block there's a clear single place to change it.

raise ArgumentError, "Reservation must be at least one day long" if check_in >= check_out
end

def validate_date_arguments(check_in: nil, check_out: nil, nights: nil)

Choose a reason for hiding this comment

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

The date validation also probably doesn't want to live in FrontDesk since that's unrelated to its job of managing the high level state of the hotel. (There are other methods that might want to be moved out as well but which ones would depend on your specific solution.)

There are two main ways to avoid repeating this in Reservation and Block:

  1. Make an Interval or DateRange class that's responsible for validation/comparison of dates.
  2. Make a parent class for both Block and Reservation that handles the above functionality.

rooms.select { |room| room.available?(nights: nights) }
end

def create_block(check_in:, check_out:, room_collection: nil, number_of_rooms: nil, room_rate:)

Choose a reason for hiding this comment

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

Most of this logic probably wants to live in Block. This method is rather coupled to the state of FrontDesk though so the challenging part would be figuring out how little you could get away with passing into the Block class.

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