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

LJ Hotel #33

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

LJ Hotel #33

wants to merge 22 commits into from

Conversation

elle-johnnie
Copy link

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 went back in forth between attempting a BookingSystem module and BookingSystem class. I ended up going the class route since I was exactly sure how to implement mixins .
Describe a concept that you gained more clarity on as you worked on this assignment. I think I gained more understanding of duck typing. The concept didn't make much sense to me prior to this assignment, but while building it and sending messages between classes I feel like I improved my understanding of the concept.
Describe a nominal test that you wrote for this assignment. A reservation can be instantiated.
Describe an edge case test that you wrote for this assignment. An argument error is raised if all rooms are booked.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I did well for the most part. I find it usually helps in the initial building stage of the project. I did find that as I was playing around with block bookings that I felt I just needed to start with code instead of tests since I wasn't sure exactly what I thought it should look like if I approached it first from the test build perspective. I likely would have benefited from pseudocoding this out, but running short on time I jumped straight into building the methods and refactoring others to get block reservations to work.

…n't figure out tests for both no method error
…_dates to attr_reader in Reservation - duh - tests are built and now working
…tion#methods to BookingSystem for better handling
@CheezItMan
Copy link

Hotel

What We're Looking For

Feature Feedback
Baseline
Used git regularly Good number of commits and good commit messages
Answer comprehension questions Check, I'm glad Duck Typing make more sense now.
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
Test coverage 99.7%
Wave 2
View available rooms for a given date range Check
Reserving a room that is not available produces an error No check for this
Wave 3
Create a block of rooms Check
Check if a block has rooms Check, inherited from Reservation
Reserve a room from a block No way to reserve a room within a block
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, but see my notes in room.rb
Appropriately uses iterators and enumerables Very well done
Appropriately writes and utilizes classes Check
Appropriately utilizes modules as a mechanism to encapsulate functionality Check
Wrap Up Will complete later
There is a refactors.txt file
The file provides a roadmap to future changes
Additional Feedback Very well done. Nice work. You hit all the learning goals and got to wave 3. I like the design and how things are laid out. See my inline notes, but very well done. You wrote some really good tests, nice work!

lib/room.rb Outdated
module Hotel
class Room
attr_reader(:id)
attr_accessor(:cost)

Choose a reason for hiding this comment

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

Do you really want cost to be an accessor?

lib/room.rb Outdated

# method to find a room
def self.find_room(id)
@id.find { |room| room.id == id }

Choose a reason for hiding this comment

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

This method doesn't make much sense because your self.all method doesn't initialize an @@id class variable, and in your @id instance variable the instance variable is just a number not a list.

(rsv_start...rsv_end).to_a.map { |day| day.to_s }
end

def get_total_cost(included_rooms, rsv_start, rsv_end, status)

Choose a reason for hiding this comment

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

You could do this method with the instance variables instead of using parameters.

Also it's neat how you have a Reservation which can have multiple rooms.

reservation.booked_dates.include? search_date.to_s
end
# search through block id's if not found in standard reservations
if found_reservations.empty?

Choose a reason for hiding this comment

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

Why do this if it doesn't find reservations during that date. Instead you would always want to find both.

end
end

found_reservations = 0 if found_reservations.empty?

Choose a reason for hiding this comment

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

Why return 0 if no reservations? Why not return an empty list?

end

# method to return total cost based on reservation id
def get_reservation_total(id)

Choose a reason for hiding this comment

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

This method seems to duplicate existing functionality.

unavailable_rooms << rsv.included_rooms
end
end
available_rooms = all_rooms_arr - unavailable_rooms.flatten

Choose a reason for hiding this comment

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

Nice!

super(input)
raise ArgumentError, 'Invalid name' if input[:group_name].nil? || input[:group_name].empty?
raise ArgumentError, 'Invalid # of rooms' if input[:room_qty].to_i < 2 || input[:room_qty].to_i > 5
@included_rooms = input[:included_rooms]

Choose a reason for hiding this comment

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

You should also be able to make reservations from within a block, after the block is "held."


def make_a_reservation(guest, check_in, check_out, number_of_rooms = 1, status = :BASE)
raise ArgumentError, 'Too many rooms' if number_of_rooms > 20 || !(number_of_rooms.is_a? Integer)
open_rooms = find_available_rooms(check_in, check_out).first(number_of_rooms)

Choose a reason for hiding this comment

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

What happens if there are not enough available rooms?

it 'can find available rooms given a check in and check out date' do
check_in = "October 23 2019"
check_out = "October 24 2019"
expect(admin.find_available_rooms(check_in, check_out)).must_be_kind_of Array

Choose a reason for hiding this comment

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

You could also check and verify each instance is a Room

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