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
Branches - Amal #39
Open
ashassan
wants to merge
25
commits into
Ada-C12:master
Choose a base branch
from
ashassan: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
Branches - Amal #39
Changes from all commits
Commits
Show all changes
25 commits
Select commit
Hold shift + click to select a range
9dddf82
created files for classes
ashassan 50b6dbf
deleted hotel block/ room and added initialize to hotel booker and re…
ashassan f1bea5a
deleted tests for hotel block/ room and added to test_helper
ashassan 69a23e0
added tests for total_cost method
ashassan a252425
added total cost method and removed date range validation
ashassan 84c9752
added code for find_available_room, added code for create_reservation…
ashassan d54afa7
added some tests for initialize, create_reservation, and find_availab…
ashassan ac5ec55
addded tests for overlap and invalid date range
ashassan b918e26
made overlap boolean zen
ashassan 30893c2
fixed available rooms method
ashassan 44b100d
added find_reservation_by_date method
ashassan 590e501
added tests for available rooms method and find reservation by date
ashassan 373f48a
removed empty initialize tests
ashassan dc6c21c
added require date
ashassan e9ba680
fixed an error in overlap
ashassan 1fde6f9
changed total cost to use instance variables
ashassan 8f88541
fixed ruby syntax
ashassan e717f79
added more tests for overlap
ashassan d2dbe95
removed unnecessary variable
ashassan 26fdfff
added a list of future changes
ashassan ff8f1b9
refactored find_reservation_by_date method
ashassan 0cee5c4
added calculate days method
ashassan 48b727e
fixed syntax and hid instance variables
ashassan ae3b084
added missing test and tests for new method
ashassan 2f1c88c
answered design activity questions
ashassan 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,42 @@ | ||
What classes does each implementation include? Are the lists the same? | ||
ShoppingCart, Order, and CartEntry are classes in both implementations. | ||
|
||
Write down a sentence to describe each class. | ||
CartEntry- calculates the price of an item added to a cart based on unit price and quantity. | ||
ShoppingCart- calculates the price of total items in a cart | ||
Order- calculates the total price of a cart including sales tax | ||
|
||
How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper. | ||
ShoppingCart holds many CartEntry objects and calculates the price of all of them. Order takes one shopping cart object and calculates final price with tax. | ||
|
||
What data does each class store? How (if at all) does this differ between the two implementations? Cart entry stores the data for the unit price and quantity. Shopping cart stores the data for all the entries. Order stores the data for the shopping cart. In the first implementation only order can get data about the price. In Implementaiton B they all have information about the price. | ||
|
||
|
||
What methods does each class have? How (if at all) does this differ between the two implementations?In implmentation A the shopping cart and cart entry have no methods. However, in implementation B they both have pricing methods. In both implementaitons Order has total price method. | ||
|
||
Consider the Order#total_price method. In each implementation: | ||
Is logic to compute the price delegated to "lower level" classes like ShoppingCart and CartEntry, or is it retained in Order? | ||
Does total_price directly manipulate the instance variables of other classes? | ||
|
||
In implementation A logic is not delegated to lower classes. However, in implementaiton b logic is delegated to lower classes. Total price does not directlymanipulate the instance variables of the other classes. | ||
|
||
If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify? | ||
|
||
You would have to add an if statement to add a discount if the quantity is higher than the bulk amount. It is easier to modify implementation B. | ||
|
||
Which implementation better adheres to the single responsibility principle?Implementation B better adheres to the single responsibility principle. | ||
|
||
|
||
Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled? | ||
Implementation B is more loosely coupled than implementation A. In implementation A the order is connected to both shoppingcart and cart entry then shopping cart is also connected to cart entry. In Implementation B order is connected to only shopping cart and shopping cart is connected to cart entry. | ||
|
||
|
||
what changes you would need to make to improve this design, and how the resulting design would be an improvement. | ||
|
||
my find_reservation_by_date is doing a lot. It is finding the days that are included in between the start date/end date. Then checking if the date is included into that. I decided to add a method in reservation that creates an array with all the dates. That way in find_reservation_by_date it only needs to check if the date is included in that array. | ||
|
||
|
||
|
||
|
||
|
||
|
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,16 @@ | ||
class Calendar | ||
attr_reader :start_date, :end_date | ||
|
||
def initialize(start_date:, end_date:) | ||
@start_date = start_date | ||
@end_date = end_date | ||
|
||
if end_date < start_date | ||
raise ArgumentError.new("Invalid Date Range (end date is before start date)") | ||
end | ||
end | ||
|
||
def overlap?(current_reservation) | ||
return !(current_reservation.start_date >= end_date || current_reservation.end_date <= start_date) | ||
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,55 @@ | ||
|
||
require_relative 'reservation' | ||
require_relative 'calendar' | ||
|
||
class NoAvailableRoomsError < StandardError | ||
end | ||
class HotelBooker | ||
attr_reader :rooms, :reservations | ||
|
||
def initialize | ||
@rooms = (1..20).map { |i| i } | ||
|
||
@reservations = {} | ||
|
||
@rooms.each do |room| | ||
@reservations[room] = [] | ||
end | ||
end | ||
|
||
def find_available_room(start_date, end_date) | ||
available_rooms = [] | ||
reservations.each do |room, reservations| | ||
if reservations.empty? | ||
available_rooms << room | ||
else | ||
date_range = Calendar.new(start_date: start_date, end_date: end_date) | ||
if reservations.all? { |reservation| !date_range.overlap?(reservation) } | ||
available_rooms << room | ||
end | ||
end | ||
end | ||
|
||
return available_rooms | ||
end | ||
|
||
def create_reservation(id, start_date, end_date) | ||
found_rooms = find_available_room(start_date, end_date) | ||
|
||
if found_rooms.empty? | ||
raise NoAvailableRoomsError | ||
else | ||
new_reservation = Reservation.new(id: id, start_date: start_date, end_date: end_date, room: found_rooms.first) | ||
reservations[found_rooms.first] = reservations[found_rooms.first] << new_reservation | ||
|
||
return new_reservation | ||
end | ||
end | ||
|
||
def find_reservation_by_date(date) | ||
reservations_for_date = reservations.map do |room, reservation| | ||
reservation.find_all { |reservation| reservation.calculate_days.include?(date)} | ||
end | ||
return reservations_for_date.flatten | ||
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,27 @@ | ||
class Reservation | ||
attr_reader :id, :start_date, :end_date, :room, :date_range,:reservation_days | ||
|
||
def initialize(id:, start_date:, end_date:, room:) | ||
@id = id | ||
@start_date = start_date | ||
@end_date = end_date | ||
@room = room | ||
end | ||
|
||
def total_cost | ||
200 * (end_date - start_date).to_i | ||
end | ||
|
||
def calculate_days | ||
days = [] | ||
current_date = end_date | ||
while current_date >= start_date | ||
days << current_date | ||
current_date -= 1 | ||
end | ||
return days | ||
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,10 @@ | ||
Refactor list | ||
- improve find_reservation_by_date by creating another method that checks if the date is included in the range | ||
-make the classes less coupled | ||
-refactor all methods to be shorter/more efficient | ||
-add a module | ||
-reword test desciptions | ||
-make method that calculates nights | ||
-check all names and make sure they make sense | ||
|
||
|
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,56 @@ | ||
require_relative "test_helper" | ||
|
||
describe "wave 2" do | ||
describe "initialize " do | ||
it "raises an argument error for invalid date range" do | ||
expect { Calendar.new(start_date: Date.new(2019, 2, 9), end_date: Date.new(2019, 2, 3)) }.must_raise ArgumentError | ||
end | ||
end | ||
describe "overlap?" do | ||
before do | ||
@reservation = Reservation.new(id: 1, start_date: Date.new(2019, 2, 5), end_date: Date.new(2019, 2, 9), room: 5) | ||
end | ||
|
||
it "returns true if new reservation endate overlaps " do | ||
date_range = Calendar.new(start_date: Date.new(2019, 2, 3), end_date: Date.new(2019, 2, 6)) | ||
expect(date_range.overlap?(@reservation)).must_equal true | ||
end | ||
|
||
it " returns true if new reserveration start date overlaps" do | ||
date_range = Calendar.new(start_date: Date.new(2019, 2, 6), end_date: Date.new(2019, 2, 11)) | ||
expect(date_range.overlap?(@reservation)).must_equal true | ||
end | ||
|
||
it " returns true if new reservation start_date and end_date both overlap" do | ||
date_range = Calendar.new(start_date: Date.new(2019, 2, 6), end_date: Date.new(2019, 2, 8)) | ||
expect(date_range.overlap?(@reservation)).must_equal true | ||
end | ||
it "returns true if new reservation includes both start_date and endate" do | ||
date_range = Calendar.new(start_date: Date.new(2019, 2, 3), end_date: Date.new(2019, 2, 14)) | ||
expect(date_range.overlap?(@reservation)).must_equal true | ||
end | ||
it "returns true if new reservation is exactly the same" do | ||
date_range = Calendar.new(start_date: Date.new(2019, 2, 5), end_date: Date.new(2019, 2, 9)) | ||
expect(date_range.overlap?(@reservation)).must_equal true | ||
end | ||
|
||
it "returns false if new reservation is completely before" do | ||
date_range = Calendar.new(start_date: Date.new(2019, 2, 1), end_date: Date.new(2019, 2, 3)) | ||
expect(date_range.overlap?(@reservation)).must_equal false | ||
end | ||
it "returns false if new reservation is completely after" do | ||
date_range = Calendar.new(start_date: Date.new(2019, 2, 11), end_date: Date.new(2019, 2, 15)) | ||
expect(date_range.overlap?(@reservation)).must_equal false | ||
end | ||
|
||
it "returns false if new reservation starts on end date " do | ||
date_range = Calendar.new(start_date: Date.new(2019, 2, 9), end_date: Date.new(2019, 2, 16)) | ||
expect(date_range.overlap?(@reservation)).must_equal false | ||
end | ||
|
||
it "returns false if new reservation ends on start date" do | ||
date_range = Calendar.new(start_date: Date.new(2019, 2, 1), end_date: Date.new(2019, 2, 5)) | ||
expect(date_range.overlap?(@reservation)).must_equal false | ||
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,108 @@ | ||
require_relative "test_helper" | ||
|
||
describe "Wave 1 " do | ||
describe "initialize" do | ||
before do | ||
@hotel = HotelBooker.new | ||
end | ||
it "creates 20 rooms " do | ||
expect(@hotel.rooms.length).must_equal 20 | ||
end | ||
it " returns rooms that are integers" do | ||
expect(@hotel.rooms).must_be_instance_of Array | ||
@hotel.rooms.each do |room| | ||
expect(room).must_be_instance_of Integer | ||
end | ||
end | ||
end | ||
|
||
describe " create_reservation" do | ||
it "returns a reservation" do | ||
hotel = HotelBooker.new | ||
reservation = hotel.create_reservation(1, Date.new(2019, 2, 3), Date.new(2019, 2, 6)) | ||
expect(reservation).must_be_instance_of Reservation | ||
end | ||
|
||
it "new reservation is added to reservations" do | ||
hotel = HotelBooker.new | ||
hotel.create_reservation(1, Date.new(2019, 2, 5), Date.new(2019, 2, 9)) | ||
expect(hotel.reservations[1].length).must_equal 1 | ||
end | ||
it "doesnt make conflicting reservations for the same room" do | ||
hotel = HotelBooker.new | ||
hotel.create_reservation(1, Date.new(2019, 2, 6), Date.new(2019, 2, 11)) | ||
hotel.create_reservation(4, Date.new(2019, 2, 7), Date.new(2019, 2, 10)) | ||
|
||
expect(hotel.reservations[1].length).must_equal 1 | ||
expect(hotel.reservations[2].length).must_equal 1 | ||
end | ||
it "new reservations can start on checkout days" do | ||
hotel = HotelBooker.new | ||
hotel.create_reservation(1, Date.new(2019, 2, 6), Date.new(2019, 2, 11)) | ||
hotel.create_reservation(2, Date.new(2019, 2, 1), Date.new(2019, 2, 6)) | ||
expect(hotel.reservations[1].length).must_equal 2 | ||
end | ||
it "produces an error if all the rooms are booked" do | ||
hotel = HotelBooker.new | ||
20.times do |i| | ||
hotel.create_reservation(i, Date.new(2019, 2, 6), Date.new(2019, 2, 11)) | ||
end | ||
expect { hotel.create_reservation(21, Date.new(2019, 2, 6), Date.new(2019, 2, 11)) }.must_raise NoAvailableRoomsError | ||
end | ||
end | ||
|
||
describe "find_reservation_by_date" do | ||
before do | ||
@hotel = HotelBooker.new | ||
@hotel.create_reservation(1, Date.new(2019, 2, 6), Date.new(2019, 2, 11)) | ||
@hotel.create_reservation(2, Date.new(2019, 2, 1), Date.new(2019, 2, 6)) | ||
@hotel.create_reservation(3, Date.new(2019, 2, 3), Date.new(2019, 2, 12)) | ||
@hotel.create_reservation(4, Date.new(2019, 2, 7), Date.new(2019, 2, 10)) | ||
@hotel.create_reservation(5, Date.new(2019, 2, 8), Date.new(2019, 2, 13)) | ||
end | ||
it "returns an array of reservations" do | ||
found_reservations = @hotel.find_reservation_by_date(Date.new(2019, 2, 10)) | ||
expect(found_reservations).must_be_instance_of Array | ||
found_reservations.each do |reservation| | ||
expect(reservation).must_be_instance_of Reservation | ||
end | ||
end | ||
it "all returned reservations inlcude the date" do | ||
found_reservations = @hotel.find_reservation_by_date(Date.new(2019, 2, 6)) | ||
expect(found_reservations[0].id).must_equal 1 | ||
expect(found_reservations[1].id).must_equal 2 | ||
expect(found_reservations[2].id).must_equal 3 | ||
expect(found_reservations[3]).must_be_nil | ||
end | ||
end | ||
end | ||
describe "Wave 2" do | ||
describe "find_available_room" do | ||
it "returns an array of available rooms" do | ||
hotel = HotelBooker.new | ||
available_rooms = hotel.find_available_room(Date.new(2019, 2, 5), Date.new(2019, 2, 9)) | ||
expect(available_rooms).must_be_instance_of Array | ||
end | ||
|
||
it "returns all the rooms when the array is empty" do | ||
hotel = HotelBooker.new | ||
available_rooms = hotel.find_available_room(Date.new(2019, 2, 5), Date.new(2019, 2, 9)) | ||
expect(available_rooms.length).must_equal 20 | ||
end | ||
|
||
it "returns only empty rooms " do | ||
hotel = HotelBooker.new | ||
|
||
hotel.create_reservation(1, Date.new(2019, 2, 6), Date.new(2019, 2, 11)) | ||
|
||
hotel.create_reservation(2, Date.new(2019, 2, 1), Date.new(2019, 2, 8)) | ||
|
||
hotel.create_reservation(3, Date.new(2019, 2, 8), Date.new(2019, 2, 12)) | ||
hotel.create_reservation(4, Date.new(2019, 2, 11), Date.new(2019, 2, 14)) | ||
hotel.create_reservation(5, Date.new(2019, 2, 11), Date.new(2019, 2, 14)) | ||
available_rooms = hotel.find_available_room(Date.new(2019, 2, 5), Date.new(2019, 2, 9)) | ||
expect(available_rooms[0]).must_equal 3 | ||
expect(available_rooms.length).must_equal 18 | ||
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,35 @@ | ||
require_relative "test_helper" | ||
|
||
describe "total_cost method" do | ||
before do | ||
@start_time = Date.new(2019, 9, 6) | ||
@end_time = Date.new(2019, 9, 9) | ||
@reservation = Reservation.new(id: 1, start_date: @start_time, end_date: @end_time, room: 5) | ||
end | ||
it "returns an instance of an integer" do | ||
expect(@reservation.total_cost).must_be_instance_of Integer | ||
end | ||
|
||
it "returns accurate calculation of total cost" do | ||
expect(@reservation.total_cost).must_equal 600 | ||
end | ||
end | ||
|
||
describe "calculate days method" do | ||
before do | ||
@start_time = Date.new(2019, 2, 1) | ||
@end_time = Date.new(2019, 2, 5) | ||
@reservation = Reservation.new(id: 1, start_date: @start_time, end_date: @end_time, room: 5) | ||
end | ||
|
||
it " returns an array" do | ||
expect(@reservation.calculate_days).must_be_instance_of Array | ||
end | ||
it "returns accurate information for days included in reservation" do | ||
days = @reservation.calculate_days | ||
expect(days.length).must_equal 5 | ||
[Date.new(2019, 2, 1), Date.new(2019, 2, 2), Date.new(2019, 2, 3), Date.new(2019, 2, 4), Date.new(2019, 2, 5)].each do |date| | ||
expect(days.include?(date)).must_equal true | ||
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,18 @@ | ||
# Add simplecov | ||
require 'simplecov' | ||
SimpleCov.start do | ||
add_filter 'test/' | ||
end | ||
require "minitest" | ||
require "minitest/autorun" | ||
require "minitest/reporters" | ||
require 'date' | ||
|
||
|
||
|
||
Minitest::Reporters.use! Minitest::Reporters::SpecReporter.new | ||
|
||
# require_relative your lib files here! | ||
require_relative '../lib/calendar' | ||
require_relative '../lib/hotel_booker' | ||
require_relative '../lib/reservation' |
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.
I love how thorough your tests are here with very specific expectations!
I'm confused though why room 3 is returned when it's already reserved starting on the 8th and the request has a checkout date of the 9th.