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

Dominique - Leaves #42

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
17 changes: 17 additions & 0 deletions lib/date_range.rb
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

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.


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
47 changes: 47 additions & 0 deletions lib/design-activity.md
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.
53 changes: 53 additions & 0 deletions lib/hotel_system.rb
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


11 changes: 11 additions & 0 deletions lib/refactors.txt
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.

Choose a reason for hiding this comment

The 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.
47 changes: 47 additions & 0 deletions lib/reservation.rb
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 added lib/room.rb
Empty file.
36 changes: 36 additions & 0 deletions test/date_range_test.rb
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
95 changes: 95 additions & 0 deletions test/hotel_system_test.rb
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

48 changes: 48 additions & 0 deletions test/reservation_test.rb
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
8 changes: 8 additions & 0 deletions test/test_helper.rb
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'