forked from AdaGold/hotel
-
Notifications
You must be signed in to change notification settings - Fork 50
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
Dominique - Leaves #42
Open
dtaylor73
wants to merge
14
commits into
Ada-C12:master
Choose a base branch
from
dtaylor73:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
e4ddc91
Date_Range instantiation
dtaylor73 3d0277d
throws an argument error if start date is after end date date_range_t…
dtaylor73 3bc0174
room instantiation room_test.rb
dtaylor73 4d3d56a
load_all method test
dtaylor73 190aac8
hotel_system instantiation tests
dtaylor73 4a8713e
list_rooms method added
dtaylor73 e7c6c60
Wave 1 complete
dtaylor73 5d9afc4
Wave 2 list_available_rooms test passed
dtaylor73 c099b5c
Wave 2 complete
dtaylor73 45840af
created refactors.txt file
dtaylor73 9e28570
final submission
dtaylor73 33bd604
created refactors.txt
dtaylor73 be2b253
design-activity.md
dtaylor73 fe02589
Hotel redesign
dtaylor73 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
module Hotel | ||
class Date_Range | ||
attr_reader :start_date, :end_date | ||
|
||
def initialize(start_date: , end_date: ) | ||
@start_date = start_date | ||
@end_date = end_date | ||
|
||
raise ArgumentError, "start date cannot be after end date" if start_date > end_date | ||
end | ||
|
||
def calculate_cost_of_stay | ||
duration_of_stay = end_date - start_date | ||
duration_of_stay.to_i * 200 | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
Question 1: What classes does each implementation include? Are the lists the same? | ||
Answer: Implementation A and Implementation B both include CartEntry, ShoppingCart and Order classes. | ||
|
||
Question 2: Write down a sentence to describe each class. | ||
Answer: Implementation A - The CartEntry class creates a single CartEntry object and defines its unit_price and quantity. The ShoppingCart class keeps track of all the CartEntry objects. The Order class calculates the total price of all the entries in the ShoppingCart by iterating through ShoppingCart's array, pulling out the unit_price and quantity attributes from each CartEntry object and then multiplying the sales_tax to the entries' subtotal. | ||
Answer: Implementation B - The CartEntry class now stores a state AND a behavior. It is instantiated with unit_price and quantity, and holds a method that calculates and returns the item/s subtotal. The ShoppingCart class keeps track of the CartEntry objects, and now holds a method that calls on CartEntry to return the sum price of all its entry objects. The Order class holds a method that returns the total price of all the entries. Now, instead of calculating the subtotal itself, the class simply asks the ShoppingCart class for the sum price of all the entry objects and then multiplies the sales_tax to that subtotal to return the total_price. | ||
|
||
Question 3: How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper. | ||
Answer: In both implementations, the Order class has one ShoppingCart object, and the ShoppingCart class has many cart entry objects. | ||
|
||
|
||
Question 4: What data does each class store? How (if at all) does this differ between the two implementations? | ||
Answer: Each class in both implementations store the same data. The CartEntry class stores unit_price and quantity. The ShoppingCart class stores an array of CartEntry objects. The Order class stores a ShoppingCart object. | ||
|
||
Question 5: What methods does each class have? How (if at all) does this differ between the two implementations? | ||
Answer: Implementation A - The CartEntry and ShoppingCart classes do not hold methods. The Order class holds one "total_price" method. | ||
Answer: Implementation B - The CartEntry and Shopping classes now each hold one method called "price." The Order class still holds one method called "total_price." | ||
|
||
Question 6: Consider the Order#total_price method. | ||
Answer: Implementation A - Logic to compute the price is retained in Order. The Order class knows too much about the ShoppingCart and CartEntry classes and also manipulates both of those classes' states (instance variables) to calculate the total_price. | ||
Answer: Implementation B - Unlike the former implementation, this version does not manipulate states of other classes. Instead, it allows the other classes to handle the logic, and then simply calls on those classes to provide an answer. | ||
|
||
Question 7: If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify? | ||
Answer: In both implementations, we'd have to alter the initialize method of the CartEntry class to determine the "unit_price" if the quantity given was over a certain amount. However, since the Order class directly manipulates the CartEntry attributes in Implementation A, we would also have to make changes to Order's "total_price" method. Therefore, implementation B is easier to modify since CartEntry is solely responsible for manipulating its own state and logic. | ||
|
||
Question 8: Which implementation better adheres to the single responsibility principle? | ||
Answer: Implementation B. Unlike Implementation A, Implementation B does not store all of the logic in the Order class. Instead it assigns logic to a class depending on which instance variables are modified. | ||
|
||
Question 9: | ||
Answer: Implementation B is more loosely coupled. Implementation A's Order class knows too much about the ShoppingCart and CartEntry classes. It has to dig through class objects to get the data it needs. If the "unit_price" and "entry" names change in CartEntry, it will break the code in Order. | ||
|
||
|
||
Hotel Re-design | ||
|
||
The Hotel_System class takes on entirely too much responsibility. I need to delegate more responsibility to the Reservation class. Instead of having Hotel_System so much logic, I will put the majority of it in | ||
Reservation. | ||
|
||
First, I plan to remove the Rooms class and make all references to room just be an integer. This will make it easier to access just room numbers, instead of digging through a room object. | ||
|
||
I plan to let Reservation do the logic to make a reservation, make a list of reserved rooms for a given | ||
date range and also specifically find the room numbers for a given reservation. | ||
|
||
I plan to let Hotel_System asks Reservation for WHAT it needs, not HOW it does it. So in each of my | ||
methods in Hotel_Sytem, the majority of them will call a Reservation method on an instance of Reservation. This way, the classes aren't so tightly coupled. | ||
|
||
I'll also let the Date_Range class calculate the cost of stay, and raise an argument error if a given | ||
start date comes after a given end date. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
|
||
require "pry" | ||
module Hotel | ||
class Hotel_System | ||
attr_reader :rooms | ||
attr_accessor :reservation | ||
|
||
def initialize(rooms: [], reservation: nil ) | ||
@rooms = rooms | ||
@reservation = reservation | ||
end | ||
|
||
def list_rooms | ||
rooms | ||
end | ||
|
||
def no_rooms_available | ||
raise ArgumentError, "There are no rooms available for that given date range." | ||
end | ||
|
||
def reservation?(room, start_date, end_date) | ||
available_room = list_available_rooms(start_date, end_date) | ||
|
||
if available_room.sample == nil | ||
no_rooms_available | ||
else | ||
reservation.make_reservation(room, start_date, end_date) | ||
end | ||
end | ||
|
||
def find_reservation(start_date, end_date) | ||
reservation.find_reservation(start_date, end_date) | ||
end | ||
|
||
def list_available_rooms(start_date, end_date) | ||
booked_rooms = list_reserved_room_numbers(start_date, end_date) | ||
|
||
available_rooms = [] | ||
rooms.each do |room| | ||
if booked_rooms.include?(room) == false | ||
available_rooms.push(room) | ||
end | ||
end | ||
available_rooms | ||
end | ||
|
||
def list_reserved_room_numbers(requested_start_date, requested_end_date) | ||
reservation.list_reserved_room_numbers(requested_start_date, requested_end_date) | ||
end | ||
end | ||
end | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
Room Class | ||
|
||
I definitely did not need a room class. My current room class made things has no useful methods. Also, since the rooms instance variable | ||
in hotel system is an array of room objects, it made it harder to access the room numbers. I ended up making unnecessary methods in hotel system | ||
like "list_available_room_numbers" because I had to extract the room numbers from the room objects. If I had no room class and | ||
simply just made the rooms instance variable in hotel system an array of integers 1 -20, I wouldn't need the extra methods. | ||
Also, there was no reason to list the room cost. Every room is $200 and total cost is calculated in the reservations class. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That seems wrong to me? Isn't it 200 per day? |
||
|
||
Use of Let | ||
|
||
Try to use let when you refactor hotel. Before is useful, but I feel like let could dry up your code. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
|
||
|
||
module Hotel | ||
class Reservation | ||
attr_reader :room, :start_date, :end_date | ||
attr_accessor :total_reservations | ||
|
||
def initialize(room: , start_date: , end_date: , total_reservations: []) | ||
@room = room | ||
@start_date = start_date | ||
@end_date = end_date | ||
@total_reservations = total_reservations | ||
end | ||
|
||
def make_reservation(room, requested_start_date, requested_end_date) | ||
total_reservations << Reservation.new( | ||
room: room, | ||
start_date: start_date, | ||
end_date: end_date) | ||
end | ||
|
||
def list_reserved_room_numbers(requested_start_date, requested_end_date) | ||
reserved_rooms = [] | ||
|
||
total_reservations.each do |reservation| | ||
if (start_date >= requested_start_date && end_date < requested_end_date) || | ||
(end_date >= requested_start_date && end_date <= requested_end_date) | ||
reserved_rooms.push(room) | ||
end | ||
end | ||
reserved_rooms | ||
end | ||
|
||
def find_reservation(requested_start_date, requested_end_date) | ||
found_reservations = [] | ||
|
||
total_reservations.each do |reservation| | ||
if (start_date >= requested_start_date && start_date < requested_end_date) || | ||
(end_date >= requested_start_date && end_date <= requested_end_date) | ||
found_reservations.push(reservation) | ||
end | ||
end | ||
found_reservations | ||
end | ||
end | ||
end | ||
|
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
require_relative "test_helper" | ||
require "date" | ||
|
||
describe "Date_Range" do | ||
before do | ||
|
||
start_date = Date.parse("2019-02-03") | ||
end_date = Date.parse("2019-02-05") | ||
|
||
@date_range = Hotel::Date_Range.new( | ||
start_date: start_date, | ||
end_date: end_date | ||
) | ||
end | ||
|
||
describe "Date_Range instantiation" do | ||
it "is an instance of Date_Range" do | ||
expect(@date_range).must_be_kind_of Hotel::Date_Range | ||
end | ||
|
||
it "throws an argument error if start date is after end date" do | ||
start_date = Date.parse("2019-02-05") | ||
end_date = Date.parse("2019-02-04") | ||
|
||
expect{Hotel::Date_Range.new(start_date: start_date, end_date: end_date)}.must_raise ArgumentError | ||
end | ||
end | ||
|
||
describe "calculate_cost_of_stay" do | ||
it "returns the cost of the duration of stay" do | ||
result = @date_range.calculate_cost_of_stay | ||
|
||
expect(result).must_equal 400 | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
require_relative "test_helper" | ||
require "minitest/skip_dsl" | ||
|
||
describe "Hotel_System" do | ||
before do | ||
rooms = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20] | ||
reservation = | ||
Hotel::Reservation.new( | ||
room: 3, | ||
start_date: Date.parse("2019-02-03"), | ||
end_date: Date.parse("2019-02-05")) | ||
|
||
@hotel_system = Hotel::Hotel_System.new(rooms: rooms, | ||
reservation: reservation) | ||
end | ||
|
||
describe "Hotel_System instantiation" do | ||
it "is an instance of Hotel" do | ||
expect(@hotel_system).must_be_kind_of Hotel::Hotel_System | ||
end | ||
|
||
it "stores an array of room numbers" do | ||
expect(@hotel_system.rooms[0]).must_be_kind_of Integer | ||
end | ||
|
||
it "stores an instance of reservation" do | ||
expect(@hotel_system.reservation).must_be_kind_of Hotel::Reservation | ||
end | ||
end | ||
|
||
describe "list_rooms" do | ||
it "returns a list of all the rooms in the hotel" do | ||
|
||
expect(@hotel_system.list_rooms).must_be_kind_of Array | ||
expect(@hotel_system.list_rooms.length).must_equal 20 | ||
expect(@hotel_system.list_rooms[0]).must_be_kind_of Integer | ||
end | ||
end | ||
|
||
describe "reservation" do | ||
it "calls the Reservation class to make a reservation when given a date range" do | ||
|
||
room = 3 | ||
start_date = Date.parse("2019-02-03") | ||
end_date = Date.parse("2019-02-05") | ||
|
||
@reservation = Hotel::Reservation.new( | ||
room: room, | ||
start_date: start_date, | ||
end_date: end_date, | ||
) | ||
|
||
@hotel_system.reservation?(3, Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
expect(@hotel_system.reservation.total_reservations.length).must_equal 1 | ||
end | ||
end | ||
|
||
describe "list_available_rooms" do | ||
it "lists available rooms for a given date" do | ||
room = 3 | ||
start_date = Date.parse("2019-02-03") | ||
end_date = Date.parse("2019-02-05") | ||
|
||
@reservation = Hotel::Reservation.new( | ||
room: room, | ||
start_date: start_date, | ||
end_date: end_date, | ||
) | ||
|
||
@hotel_system.reservation?(3, Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
@hotel_system.list_reserved_room_numbers(Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
result = @hotel_system.list_available_rooms(Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
expect(result.length).must_equal 19 | ||
end | ||
end | ||
|
||
describe "list_reserved_room_numbers" do | ||
it "returns reserved room numbers" do | ||
room = 3 | ||
start_date = Date.parse("2019-02-03") | ||
end_date = Date.parse("2019-02-05") | ||
|
||
@reservation = Hotel::Reservation.new( | ||
room: room, | ||
start_date: start_date, | ||
end_date: end_date, | ||
) | ||
|
||
@hotel_system.reservation?(3, Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
result = @hotel_system.list_reserved_room_numbers(Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
expect(result[0]).must_equal 3 | ||
end | ||
end | ||
end | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
require_relative "test_helper" | ||
|
||
describe "Reservation" do | ||
before do | ||
|
||
room = 3 | ||
start_date = Date.parse("2019-02-03") | ||
end_date = Date.parse("2019-02-05") | ||
|
||
@reservation = Hotel::Reservation.new( | ||
room: room, | ||
start_date: start_date, | ||
end_date: end_date, | ||
) | ||
end | ||
|
||
describe "Reservation instantiation" do | ||
it "is instance of Reservation" do | ||
expect(@reservation).must_be_kind_of Hotel::Reservation | ||
end | ||
end | ||
|
||
describe "make_reservation" do | ||
it "makes and pushes reservations into the total_reservations array" do | ||
@reservation.make_reservation(3, Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
expect(@reservation.total_reservations.length).must_equal 1 | ||
end | ||
end | ||
|
||
describe "list_reserved_room_numbers" do | ||
it "returns an array of reserved room numbers for a given date range" do | ||
@reservation.make_reservation(3, Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
|
||
result = @reservation.list_reserved_room_numbers(Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
expect(result[0]).must_equal 3 | ||
end | ||
end | ||
|
||
describe "find_reservation" do | ||
it "returns an array of all the reservation instances" do | ||
@reservation.make_reservation(4, Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
@reservation.make_reservation(5, Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
|
||
result = @reservation.find_reservation(Date.parse("2019-02-03"), Date.parse("2019-02-05")) | ||
expect(result.length).must_equal 2 | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,16 @@ | ||
# Add simplecov | ||
require "simplecov" | ||
SimpleCov.start | ||
|
||
require "minitest" | ||
require "minitest/autorun" | ||
require "minitest/reporters" | ||
require "minitest/skip_dsl" | ||
|
||
Minitest::Reporters.use! Minitest::Reporters::SpecReporter.new | ||
|
||
# require_relative your lib files here! | ||
require_relative '../lib/hotel_system' | ||
require_relative '../lib/reservation' | ||
require_relative '../lib/date_range' | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use a class without any methods besides
initialize
, you probably either don't need a class or your other classes are doing work that belongs to the empty class.