Skip to content

maryams_hotel#45

Open
marshi23 wants to merge 10 commits intoAda-C10:masterfrom
marshi23:master
Open

maryams_hotel#45
marshi23 wants to merge 10 commits intoAda-C10:masterfrom
marshi23:master

Conversation

@marshi23
Copy link

@marshi23 marshi23 commented Sep 10, 2018

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 choose to make three classes, Hotel, Reservation, and Room. I was considering not having Hotel but I decided to have three class. I was also considering a BlockReservation class down the road, it could be an independent class or a child of Reservation. There was not time to pursue all of that though. At the end if the day, I think three class is the most ideal for me.
Describe a concept that you gained more clarity on as you worked on this assignment. I walked away with a better understanding of how to connect and relate classes with one another. I got more practice using and creating class variables and methods in relation to these connections. It also helped me identify problem areas and strengths. I learnt the importance of breaking down tasks, and starting simple.
Describe a nominal test that you wrote for this assignment. "returns an instance of Hotel" is one of my nominal tests, it checks to confirm that calling a new Hotel returns and instance of Hotel.
Describe an edge case test that you wrote for this assignment. I unfortunately did not have edge case tests, I'm still learning to work in ttd.
How do you feel you did in writing pseudocode first, then writing the tests and then the code? I enjoyed the process and I thought it was really helpful. I spent too much time in design and pseudocode phase, I like pseudocode, it gives me structure. And it allows me to go on tangents to explore ideas. Tests were hard to maintain.

@CheezItMan
Copy link

CheezItMan commented Sep 17, 2018

Hotel

What We're Looking For

Feature Feedback
Baseline
Used git regularly Considering things, not many commits, decent commit messages, but try to be more descriptive than, a few fixes.
Answer comprehension questions Check
Design
Each class is responsible for a single piece of the program For the mot part, however Room and Hotel seem to have overlapping responsibilities.
Classes are loosely coupled Reservation is pretty tightly coupled to everything.
Wave 1
List rooms Check
Reserve a room for a given date range Check
List reservations for a given date Missing You have a method which will find one reservation given a date range, but not all the reservations in that time period.
Calculate reservation price Check
Invalid date range produces an error Check
Test coverage 85%
Wave 2
View available rooms for a given date range Check
Reserving a room that is not available produces an error Missing it just fails by going beyond the bounds of the array.
Test coverage 85%
Wave 3 INCOMPLETE
Create a block of rooms
Check if a block has rooms
Reserve a room from a block
Test coverage
Fundamentals
Names variables, classes and modules appropriately Pretty good, see my notes on class names.
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 No class methods used, which is good
Appropriately uses iterators and enumerables Check
Appropriately writes and utilizes classes Check
Appropriately utilizes modules as a mechanism to encapsulate functionality Check
Wrap Up
There is a refactors.txt file MISSING
The file provides a roadmap to future changes MISSING
Additional Feedback Some really good stuff here. Your general design is workable and you had good use of Enumerables. I do encourage you to look at Marshi23 and my comments and see areas for improvement. It is clear that you've spent a lot of time going back and forth with design elements. It can help quite a bit to grab a whiteboard and talk through a design with someone. Overall not bad, lots of things to improve, but that's normal with this project.

@@ -0,0 +1,22 @@
class Room

Choose a reason for hiding this comment

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

This class seems to mostly serve as a a container for reservations, which might be better served as a feature of your Hotel class.

room = rooms_available(check_in, check_out).first

reservation = Reservation.new(check_in, check_out, room)
@reservations << reservation

Choose a reason for hiding this comment

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

It's a little awkward that your Hotel and your Room objects both need to track the Reservation. It would be simpler if the Reservation was just stored in Hotel.

binding.pry
end
end

Choose a reason for hiding this comment

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

You've only tested the initialize method. You should also test:

  1. That you can make a reservation
  2. That if you have a reservation in a date range, the next room is selected.
  3. If all rooms are booked, an error is produced.
  4. If a reservation ends on the new booking date, the room can be reserved.

it "calculates the reservation cost correctly" do
expect(@reservation.cost).must_equal 600
end

Choose a reason for hiding this comment

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

Since it's public, you should also test the nights method.

Plus the during? method.

end

it "raises an error if room number is higher than 20" do
expect{(Room.new(21))}.must_raise ArgumentError

Choose a reason for hiding this comment

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

Do you really need to have a test for this here? It makes the Room class harder to reuse.

Should Room raise an error if the number is higher than 20?

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.

3 participants