Skip to content

Comments

hotel project - jessie zhang#25

Open
jessiezhang2017 wants to merge 22 commits intoAda-C10:masterfrom
jessiezhang2017:master
Open

hotel project - jessie zhang#25
jessiezhang2017 wants to merge 22 commits intoAda-C10:masterfrom
jessiezhang2017:master

Conversation

@jessiezhang2017
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 need to decide if I want to create a child class Block to inherit the Admin class , to help solve the wave 3 room-block issue. Or, I can just create methods within the Admin class to solve the problem. I decide to create a Block class , since looks it can save me some time . I also need to decide if I want to generate an instance attribute to hold a list of the available room with date hash. or, I can work on the list of reservations directly. I decide to create the list of available room with data hash. It seems more direct, and can help to build create room-block method in wave three.
Describe a concept that you gained more clarity on as you worked on this assignment. I understand more how to use inheritance .
Describe a nominal test that you wrote for this assignment. for create_room_block method, a nominal test would be returns a new object of Block , or remove the related rooms from the room_unbooked_dates array in the specified date
Describe an edge case test that you wrote for this assignment. an_edge case would be raise ArgumentError if the period requested for set up room block is outside of the current working period.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I did not have that habit before, but after I tried, I think it help to make me thinking more thoroughly before I start to write the tests.

@CheezItMan
Copy link

Hotel

What We're Looking For

Feature Feedback
Baseline
Used git regularly Good number of commits, but you sometimes use waves in your commit messages. Stick to describing the functionality added. Otherwise good commit messages
Answer comprehension questions Check
Design
Each class is responsible for a single piece of the program Check, although Block can do Admin functions which makes little sense.
Classes are loosely coupled Block and Admin are tightly coupled
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 98%
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, but awkwardly done with inheritance
Check if a block has rooms Check
Reserve a room from a block Check, but it's using Admin functionality
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 enumerables Check
Appropriately writes and utilizes classes Check, but I think you are misusing inheritance
Appropriately utilizes modules as a mechanism to encapsulate functionality No module used
Wrap Up
There is a refactors.txt file MISING
The file provides a roadmap to future changes MISING
Additional Feedback Creative solution to keeping a list of dates. It's not space-efficient, but somewhat easier to code. You have some pretty good testing. You also have some good breakout of functionality, but as I noted several times making Block a child class of Admin, is a little odd. I left some comments inline in your code, let me know if you have questions. Overall well done.

lib/admin.rb Outdated
end

# make new reservations
def make_reservation(reservation_id, customer_name, start_date, end_date)

Choose a reason for hiding this comment

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

Why do you need a reservation_id to make a reservation?


# input a string of date, to return the list of the reservations on that date
def list_reservations(date_selected)
return @reservations.select {|reserve| reserve.dates_booked.include? date_selected}

Choose a reason for hiding this comment

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

👍

require_relative 'room'
require_relative 'admin'

class Block < Admin

Choose a reason for hiding this comment

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

Is a Block and Admin? Why make it a subclass?

lib/main.rb Outdated
@@ -0,0 +1,19 @@
require 'pry'

Choose a reason for hiding this comment

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

This file is unnecessary in the project.


it "will not create a reservation if no room is available" do

expect(@admin_1.reservations.length).must_equal 22

Choose a reason for hiding this comment

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

This test isn't trying to create a reservation


end

it "returns nil if no result being found" do

Choose a reason for hiding this comment

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

not nil an empty array.


it "if result being found, must return an array of resrvations " do
expect(@admin_1.list_reservations(Date.new(2018,12,5))).must_include @res_1
expect(@admin_1.list_reservations(Date.new(2018,12,5))).must_include @res_2

Choose a reason for hiding this comment

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

You should also verify that they're all reservations.

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