hotel- Leanne Rivera by leannerivera · Pull Request #32 · Ada-C10/hotel

leannerivera · 2018-09-10T15:03:03Z

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?	At first I was doing more classes with inheritance. It worked for some classes, but with others the classes would start breaking. I also at first wanted a hotel class. I decided that it wasn't necessary, I just needed the admin and perhaps the rooms, and a module to hold everything together.
Describe a concept that you gained more clarity on as you worked on this assignment.	Modules. I kind of understood them before but after this project and reading more on them, I understand them much better, especially when it comes to module methods, etc.
Describe a nominal test that you wrote for this assignment.	that if a room is reserved the dates are updated in the respective hash
Describe an edge case test that you wrote for this assignment.	passing in a string instead of a date will error
How do you feel you did in writing pseudocode first, then writing the tests and then the code?	it is good, helps with the thinking process and can also help with design.

CheezItMan · 2018-09-17T02:11:40Z

Hotel

What We're Looking For

Feature	Feedback
Baseline
Used git regularly	Good number of commits and awesome commit messages! Nice work!
Answer comprehension questions	Check, I'm glad you understand modules better now.
Design
Each class is responsible for a single piece of the program	No, Concierge and the Lodging module seem to be doing almost everything.
Classes are loosely coupled	No very tightly coupled.
Wave 1
List rooms	Check
Reserve a room for a given date range	Check, but I don't think it works if you try to make more than 20 reservations, given different date ranges. That's because each room has one status value, not a status depending on the date.
List reservations for a given date	Sort of, it lists rooms that are reserved by date.
Calculate reservation price	with `receipt` method
Invalid date range produces an error	Check
Test coverage	100%
Wave 2
View available rooms for a given date range	Check, but see my inline notes
Reserving a room that is not available produces an error	No check for when no room is available
Wave 3
Create a block of rooms	Check
Check if a block has rooms	Check
Reserve a room from a block	Check
Fundamentals
Names variables, classes and modules appropriately	Somewhat, 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	Check
Appropriately uses iterators and enumerables	Check
Appropriately writes and utilizes classes	Check, but you really need some sort of class to encapsulate the concept of a `Reservation` and `Block`.
Appropriately utilizes modules as a mechanism to encapsulate functionality	Check
Wrap Up
There is a refactors.txt file	Check, good insight into the `Concierge` class.
The file provides a roadmap to future changes	Somewhat, the details of `moving around` are unclear.
Additional Feedback	Not bad, you have functionality in place, but waves 2-3 don't really fully work and aren't fully tested. If a room is reserved from 10/10/2018 to 10/15/2018 it is available on 10/15/2018 to 10/18/2018, but with your `status` attribute it won't be and you never tested that scenario. Take a look at my inline notes and let me know if you have questions.

CheezItMan · 2018-09-17T01:47:59Z

lib/concierge.rb

+
+module Lodging
+
+  class Concierge


Love the name, but maybe describe the functionality more clearly. BookingManager or something similar.

CheezItMan · 2018-09-17T01:48:31Z

lib/room.rb

+module Lodging
+
+  class Room
+    attr_reader :room_number, :status, :cost


Does a room need a status?

CheezItMan · 2018-09-17T01:49:07Z

lib/room.rb

+      room = [@room_number, @cost, @status, Array.new]
+
+      CSV.open('data/all_hotel_rooms.csv', 'a+') do |row|
+          row << room


Your initialize method is reading in all the hotel rooms for each hotel room. This doesn't seem to make sense.

CheezItMan · 2018-09-17T01:49:46Z

lib/room.rb

+    end
+
+    def self.available_room #to return one room with status available
+      CSV.open('data/all_hotel_rooms.csv', 'r', headers: true) do |row|


This just reads in the CSV file and returns the 1st row. That doesn't make sense to me given the method name.

CheezItMan · 2018-09-17T01:54:47Z

lib/concierge.rb

+
+      raise ArgumentError if Date.parse(check_out) < Date.parse(check_in) #checkout cannot be earlier than check in
+
+      book_this = Lodging.room_status(@rooms_info, check_in) #returns one available room


What is the purpose of Lodging if your Concierge is keeping track of the rooms. Why not give it the job of finding an available room.

Also why name the method room_status instead of available_room.

CheezItMan · 2018-09-17T01:55:54Z

lib/concierge.rb

+        room[:room_number] == book_this[:room_number]
+      end #finds room to be reserved
+
+       update_room[:status] = "unavailable" #changes status of said room in the array of hashes as 'unavailable'


A room isn't going to have 1 status. A room could be available in one date range and unavailable in another.

i thought room status checked for this with the conditional.

CheezItMan · 2018-09-17T01:56:52Z

lib/concierge.rb

+      block.times do
+        hold_this = Lodging.room_status(@rooms_info, check_in) #returns one available room
+
+        update_room = @rooms_info.find do |room|


This looks like a good candidate for a helper method.

CheezItMan · 2018-09-17T01:58:09Z

lib/concierge.rb

+
+        new_cost = (update_room[:cost].to_f) - (update_room[:cost].to_f * discount)
+
+        update_room[:cost] = new_cost.round(2).to_s


If you're giving a room a different cost per reservation, that value should apply to each reservation or booking, not to the room itself. A room could have a default cost, but now if a room is put into a block, the cost for past reservations go down.

CheezItMan · 2018-09-17T01:59:34Z

lib/lodging.rb

+require 'awesome_print'
+
+module Lodging
+


These methods look like they should go into Concierge instead of a separate module.

CheezItMan · 2018-09-17T02:01:09Z

spec/concierge_spec.rb

+
+      expect(tiny_hotel.all_rooms[0][:reserved_dates].length).must_equal 7
+
+    end


What about testing if you can do multiple reservations.

What happens if you reserve rooms and no room is left available.

These things should be tested!

leannerivera added 12 commits September 4, 2018 13:41

barebones created

c8510d8

barebones done

1985514

hotel class and room class can initialize, can list rooms

a88dc13

complete rehaul

4f279b5

can create rooms, assign each number - tests passed

04272bc

can view all rooms / for spec, class var cleared before tests

714e9d1

can check availability

d384892

now data stored in csvs

33f563f

ignore folder done

211253f

can receipt, can view reserved by date

c76b657

can block reserve, can block hold

343e980

refactors.txt

2a0ac2b

CheezItMan reviewed Sep 17, 2018

View reviewed changes

leannerivera added 6 commits September 30, 2018 15:43

design activity md file added

05bfb45

prompts done

82e7f9f

clearer md

0fac536

fixed

1f99ebc

removed some unneccessary csv stuff

8062098

module methods removed

b4769f7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hotel- Leanne Rivera#32

hotel- Leanne Rivera#32
leannerivera wants to merge 18 commits intoAda-C10:masterfrom
leannerivera:master

leannerivera commented Sep 10, 2018

Uh oh!

CheezItMan commented Sep 17, 2018

Uh oh!

CheezItMan Sep 17, 2018

Uh oh!

CheezItMan Sep 17, 2018

Uh oh!

CheezItMan Sep 17, 2018

Uh oh!

CheezItMan Sep 17, 2018

Uh oh!

CheezItMan Sep 17, 2018

Uh oh!

CheezItMan Sep 17, 2018

Uh oh!

leannerivera Oct 1, 2018

Uh oh!

CheezItMan Sep 17, 2018

Uh oh!

CheezItMan Sep 17, 2018

Uh oh!

CheezItMan Sep 17, 2018

Uh oh!

CheezItMan Sep 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		raise ArgumentError if Date.parse(check_out) < Date.parse(check_in) #checkout cannot be earlier than check in

		book_this = Lodging.room_status(@rooms_info, check_in) #returns one available room


		new_cost = (update_room[:cost].to_f) - (update_room[:cost].to_f * discount)

		update_room[:cost] = new_cost.round(2).to_s


		expect(tiny_hotel.all_rooms[0][:reserved_dates].length).must_equal 7

		end

Conversation

leannerivera commented Sep 10, 2018

Hotel

Comprehension Questions

Uh oh!

CheezItMan commented Sep 17, 2018

Hotel

What We're Looking For

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants