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

Leaves - Tiffany #25

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

Leaves - Tiffany #25

wants to merge 38 commits into from

Conversation

TiffanyChio
Copy link

Hotel

Congratulations! You're submitting your assignment!

Comprehension Questions

Question Answer
What was a design challenge that you encountered on this project? I feel that up until now, the focus has been on code readability, even if it requires sacrificing memory efficiency or time efficiency. With this project the focus may have flipped?
What was a design decision you made that changed over time over the project? I included a Date class in my design and it became the core of my code. However it appears to be not entirely necessary. I could have relied more heavily on iterating through my list of reservations to get the information that I needed. By the time, I realized this it was too late to change the code without starting over from scratch because of how central the Date class was to my code. However I still like the design. I think it is intuitive and readable.
What was a concept you gained clarity on, or a learning that you'd like to share? I initially called Dates HotelDates. But modules are namespaces and I wanted to use the Hotel module appropriately. It was still causing conflicts with Ruby's built-in Date class. Chris recommended using ::Date, which does not specify a module and Ruby was able to differentiate between the two classes.
What is an example of a nominal test that you wrote for this assignment? What makes it a nominal case? Reservations do not impact list of available rooms outside of date range. The dates are no where near overlap in this case.
What is an example of an edge case test that you wrote for this assignment? What makes it an edge case? Reservations can be made if the check-in date is overlaps with the check-out date of another reservation for the same room. This is an edge case because there is an overlap in dates though rooms are available for check-in on the same day as another check-out.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I pseudocoded my tests first and then wrote my code. I did it but I definitely did not enjoy it. Writing tests after writing the code feels more intuitive to me. It would also help me better create edge cases to test.

…tions overall. Created and passed all reservation initialization tests.
@beccaelenzil
Copy link

Hotel

What We're Looking For

Test Inspection

Workflow yes / yes but no test / no
Wave 1
List rooms yes
Reserve a room for a given date range yes
Reserve a room (edge case) yes
List reservations for a given date yes
Calculate reservation price yes
Invalid date range produces an error yes
Test coverage 100% !
Wave 2
View available rooms for a given date range yes
Reserving a room that is not available produces an error yes
Test coverage 100% !
Wave 3
Create a block of rooms yes
Check if a block has rooms yes
Reserve a room from a block yes
Test coverage 100% !

Code Review

Baseline Feedback
Used git regularly yes
Answer comprehension questions yes
Design
Each class is responsible for a single piece of the program yes
Classes are loosely coupled 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 -- nice work on this front
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

Overall Feedback

Great work overall! You've built your first project with minimal starting code. This represents an incredible milestone in your journey, and you should be proud of yourself! In particular nice work implementing Wave 3. Your use of inheritance for HotelBlock and the change_status method are quite clever.

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

Choose a reason for hiding this comment

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

This is a great example of defining your own appropriately named exception type.


# Returns only reservations and not hotel blocks as per specs.
def list_reservations
return @occupied.values.select { |value| value.class == Hotel::Reservation }

Choose a reason for hiding this comment

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

Ruby will be looking for names locally first, prioritizing local names, since they're all in the same module, there's no need to namespace Hotel::Reservation. Reservation should just work! Same with the rest of the namespacing in the code.


# Returns only reservations and not hotel blocks as per specs.
def list_reservations
return @occupied.values.select { |value| value.class == Hotel::Reservation }

Choose a reason for hiding this comment

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

clever use of select!

require_relative 'reservation'

module Hotel
class HotelBlock < Reservation

Choose a reason for hiding this comment

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

Great use of inheritance.

attr_reader :rooms, :reservations, :dates, :hotelblocks

def initialize
@rooms = Hotel::Room.generate_rooms

Choose a reason for hiding this comment

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

Nice use of a class method.

def initialize
@rooms = Hotel::Room.generate_rooms
@reservations = []
@dates = []

Choose a reason for hiding this comment

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

A comment to tell us what @dates is meant to hold would be useful, as it's not immediately obvious.

expect(@first_res).must_be_instance_of Hotel::Reservation

expect(@first_res.id).must_equal 1
expect(@first_res.start_date).must_equal ::Date.parse('2019-06-05')

Choose a reason for hiding this comment

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

Consider assigning dates to variables in your tests to make it clear what you are testing.

Suggested change
expect(@first_res.start_date).must_equal ::Date.parse('2019-06-05')
start_date = Date.parse('2019-06-05')
end_date = start_date + 3

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