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

Sockets - Erica #25

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

Sockets - Erica #25

wants to merge 34 commits into from

Conversation

norrise120
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 faced was determined exactly how much Room, Reservation, and Block should know about each other. I knew pretty early on that I did not want Room to know about Reservations or Block, but I went back and forth about Block knowing about reservations and vice versa. I ultimately decided that the two would not know about each other, in trying to maintain SRP. This result in a limitation in how I checked for available rooms in that I had to go through both the reservations array and block array in the hotel manager class.
What was a design decision you made that changed over time over the project? One thing that I started playing with early on was having a separate helper class that would house different methods that I thought would be useful to multiple classes (the parse dates method was one example). I ultimately got rid of this class after discussing with Dan what was being implied by having this helper class. In my original code the classes that accessed the helper class did so by inheriting. This was a relationship that ultimately did not make sense (as neither the hotelmanager nor the reservation class types of "helper").
What was a concept you gained clarity on, or a learning that you'd like to share? Even though I ultimately did not use inheritance in my code, this project really helped me gain a better understanding of inheritance. First, as above, I got a better understanding of what is being implied by inheritance. I also learned, that in inheriting, you are accepting everything from the parent class into the child class, which is why I didn't have my block class inherit from the reservation class as I didn't want the "room" variable from the reservation class to come over to the block class.
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 that I wrote was testing to make sure that a room can be instantiated if given good input. This is a nominal case because it is something that will happen regularly.
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case? Reviewing my code, I'm not sure I actually wrote any edge case tests. I do have some tests that except ArgumentErrors, but I believe those count as nominal cases. This is something I would like to improve during the refactoring process.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I think this was especially helpful in the beginning. However, admittedly, once my code started getting more complicated, particularly with the different block methods, I switched to writing my code and the tests. I think I made the switch because it got hard to figure out what the arrange would look like when I need to call a few methods. This may have been solved by doing a little more in-depth pseudocoding so I could have a better understanding of what my code was going to look out before writing the tests and then the code.

…e, wrote code to pass, updated Reservation constructor
… by creating make_reservation and list_reservations_by_date methods
…and updated constructor and make_reservation to use new method
@droberts-sea
Copy link

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
Classes are loosely coupled
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 yes
Appropriately uses iterators and Enumerable methods yes
Appropriately writes and utilizes classes yes
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 Great job overall. This code is well-organized, well-written and well-tested, and does a good job of solving the problem at hand. Keep up the hard work!

def create_hash_of_rooms_with_status(rooms)
hash = Hash[rooms.collect { |room| [room, "AVAILABLE"] }]
return hash
end

Choose a reason for hiding this comment

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

This is a clever way to think about this problem.

found_reservations = []
@reservations.each do |reservation|
if date >= reservation.start_date && date < reservation.end_date
found_reservations.push(reservation)

Choose a reason for hiding this comment

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

The .select enumerable might be helpful to clean up this code.

available_rooms = @rooms
reservations.each do |reservation|
if reservation.start_date >= start_date && reservation.end_date <= end_date
available_rooms.delete(reservation.room)

Choose a reason for hiding this comment

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

Here the Hotel goes in and looks at the details of the Reservation class (its start date and end date), which is not great design. Hotel needs to know a lot about Reservation for that to work, and the two classes are more tightly coupled than they need to be. An alternative approach would be to write a method Reservation#overlaps?(start_date, end_date), and call that here instead:

reservations.each do |reservation|
  if reservation.overlaps?(start_date, end_date)
    available_rooms.delete(reservation.room)
  end
end

I know we hadn't read POODR ch 4 yet when you wrote this code, but this is what Metz is talking about when she says you should "ask for what instead of telling how".

def calculate_cost
total_cost = ((@end_date - @start_date) - 1) * @room.cost_per_night
return total_cost
end

Choose a reason for hiding this comment

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

I like that you use a method for this rather than storing it in an instance variable. That way if the number of nights or cost per night changes, the cost will always be correct.

it "doesn't include reservations where the end date is the date" do
reservations = @hotel_manager.list_reservations_by_date("2019-3-20")

expect(reservations.length).must_equal 0

Choose a reason for hiding this comment

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

I would say this counts as an edge case - you're testing for the "edge" between two different behaviors this method can exhibit.

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