forked from AdaGold/hotel
-
Notifications
You must be signed in to change notification settings - Fork 45
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
Ampers - Abinnet #41
Open
Abiaina
wants to merge
29
commits into
Ada-C9:master
Choose a base branch
from
Abiaina: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
Ampers - Abinnet #41
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
963ba6c
tests written and passed for room and admin class
Abiaina 881a7d5
room, admin, reservation classes passing tests; most of wave 1 completed
Abiaina 48a4ce0
completed admin code for generating new reservations, missing adequat…
Abiaina 5860f19
some tests pass, missing wave 1 price at reservation and booking inqu…
Abiaina 5531ef5
passes test, most of wave 1 completed
Abiaina 224e90e
admin and reservation decoupled, confirms valid date entry for reserv…
Abiaina c166e23
reservation class instantiated
Abiaina 1a453d0
removed room instances from admin, updated testing for admin to take …
Abiaina 58a770e
calculate reservation cost in reservation cost, testing for calling c…
Abiaina d7a17f8
books reservation from 2 dates, assigns room, testing confirms reserv…
Abiaina dd6e2d1
can search for reservations with check-in/check-out range that inters…
Abiaina 46f2d1c
removed room class
Abiaina d0f6177
return room id by reservation, corrected testing for reservations arr…
Abiaina e659624
testing for availble rooms by date outline
Abiaina 467cf83
determines available rooms for certain date ranges, test written and …
Abiaina 45dbe06
available_rooms logic fixed, tests updated and passing
Abiaina 81e50a1
outline testing for updates to add_reservation, compares new reservat…
Abiaina b2adbf4
completed most of testing for making new reservation without conflict…
Abiaina 8782399
argument error raised for reservations attempted in fully booked hotel
Abiaina 317c6f7
testing completed for preventing double assigning the same room
Abiaina 47cf59b
raises argument for no available rooms reservation attempt, reassigns…
Abiaina ca58274
raises argument error when attempting to book a reserved room, does n…
Abiaina d6ecd15
block class created and testing for block class started
Abiaina cab4d12
admin can block rooms, set variable rate, reserve rooms in and out of…
Abiaina 96f08c2
error fixed using .dup, removed code that removes list of blocked roo…
Abiaina ea49bb3
answered design-activity prompts
Abiaina 19c7d16
created outline for design improvements decoupling the reservation an…
Abiaina d9a1374
removed instance variable price from admin class
Abiaina d44fc34
moved logic for new reservation of block into block_room class
Abiaina 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
Binary file not shown.
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,9 @@ | ||
require 'rake/testtask' | ||
|
||
Rake::TestTask.new do |t| | ||
t.libs = ["lib"] | ||
t.warning = true | ||
t.test_files = FileList['specs/spec_*.rb'] | ||
end | ||
|
||
task default: :test |
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 @@ | ||
What classes does each implementation include? Are the lists the same? | ||
Both A & B have classes: CartEntry, ShoppingCart, and Order. Yes, the list of classes are the same for both implementations. | ||
|
||
Write down a sentence to describe each class. | ||
CartEntry: Tracks the quantity of each item ordered and their individual item costs. B has a method to calculate cost of items. | ||
|
||
ShoppingCart: Tracks the items ordered, can calculate the subtotal or order. B has a method to calculate running subtotal. | ||
|
||
Order: Calculates the total for each order, B does relies on its other classes and their methods to do the logic required. | ||
|
||
How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper. | ||
Order class creates a shopping cart that has an instance variable entries that can store instances of Cart entries. This is then used to calculate total price with the order class' total_price instance method. | ||
|
||
|
||
What data does each class store? How (if at all) does this differ between the two implementations? | ||
CartEntry: stores number quantity of an item and item unit price. Implementation B also has a method to return the price accounting only for item quantity. | ||
|
||
ShoppingCart: Stores instance of CartEntry. Implementation B calculates subtotal of all items in a cart. | ||
|
||
Order: creates instances of ShoppingCart and calculates the total_price of all items. ShoppingCart is an array that can hold instances of CartEntry. | ||
|
||
|
||
What methods does each class have? How (if at all) does this differ between the two implementations? | ||
CartEntry & ShoppingCart Implementation A: no instance methods, except for initialize. | ||
|
||
CartEntry & ShoppingCart Implementation B: has 2 helper methods to calculate subtotal of instance variables values. | ||
|
||
Order Implementation A & B: Stores instance of CartEntry. Implementation B calculates subtotal of all items in a cart. | ||
|
||
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? | ||
Implementation A: It is computed in Order. | ||
|
||
Implementation B: It is completed in all the classes. Total base price per item in CartEntry. Subtotal for all entries in ShoppingCart is called in ShoppingCart. Order does the final calculates the total or the order. | ||
|
||
Does total_price directly manipulate the instance variables of other classes? | ||
If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify? | ||
Implementation A: Yes it acts on the CartEntry class using instance variables unit_price and quantity. | ||
|
||
Implementation B: Yes it acts directly on the ShoppingCart instance using the price instance method of the ShoppingCart class. | ||
|
||
Which implementation better adheres to the single responsibility principle? | ||
Implementation B because it only calculates the total price based on the subtotal returned by the ShoppingCart price instance method and shares the logic required for calculating the total across all of the classes. | ||
|
||
Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled? | ||
|
||
Implementation B because it separates uses a local variable subtotal and instance variable #@cart to shield the ShoppingCart instance from the calculation done by total_price method of Order. | ||
|
||
|
||
Revisiting Hotel Activity | ||
There are a couple of places where I violate the single responsibility rule for classes. | ||
|
||
I chose to remove the instance variable price in admin and use a local variable to store the block room prices. This did not require any changes to my testing. | ||
|
||
I also decided to change the block_add_reservation instance method in admin and move the logic and making of new reservation into block_room class. The tests pass still. |
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,94 @@ | ||
require 'date' | ||
require_relative 'reservation' | ||
require_relative 'block_room' | ||
|
||
class Admin | ||
attr_reader :roomlist, :reservations, :blocks | ||
|
||
def initialize | ||
@roomlist = [] | ||
@reservations = [] | ||
|
||
#array of block instances | ||
@blocks = [] | ||
|
||
20.times do |i| | ||
@roomlist << (i + 1) | ||
end | ||
end | ||
|
||
def add_reservation(check_in, check_out, room_id) | ||
available_rooms = available_rooms(check_in, check_out) | ||
|
||
if !(available_rooms.include?(room_id)) | ||
raise ArgumentError.new("#{room_id} is booked for these dates") | ||
end | ||
|
||
if available_rooms.count == 0 | ||
raise ArgumentError.new("No available roooms") | ||
end | ||
|
||
@reservations << Reservation.new(check_in, check_out, room_id) | ||
end | ||
|
||
|
||
def available_rooms(check_in, check_out) | ||
|
||
check_in = Date.parse(check_in) | ||
check_out = Date.parse(check_out) | ||
|
||
rooms = @roomlist.dup | ||
|
||
@reservations.each do |reservation| | ||
valid_check_in = check_in.between?(reservation.start_date, (reservation.end_date - 1)) | ||
|
||
|
||
valid_check_out = check_out.between?((reservation.start_date + 1), reservation.end_date) | ||
|
||
if valid_check_in && valid_check_out | ||
rooms.delete(reservation.room_id) | ||
end | ||
end | ||
return rooms | ||
end | ||
|
||
def new_block(check_in, check_out, number_of_rooms, rate) | ||
block_rooms_ids = [] | ||
|
||
rooms_available = available_rooms(check_in, check_out) | ||
|
||
if number_of_rooms <= 5 && rooms_available.count >= number_of_rooms | ||
number_of_rooms.times do | ||
block_rooms_ids << rooms_available[0] | ||
end | ||
|
||
block = BlockRoom.new(check_in, check_out, rate, block_rooms_ids) | ||
|
||
@blocks << block | ||
else | ||
raise ArgumentError.new("Can only block 5 rooms at a time. Can only block available rooms. There are #{available_rooms.count} rooms.") | ||
end | ||
end | ||
|
||
# Modified this method | ||
def block_add_reservation(block) | ||
new_rez = block.reserve_block | ||
|
||
@reservations << new_rez | ||
end | ||
|
||
def bookings_by_date(date) | ||
bookings_by_day = [] | ||
|
||
date = Date.parse(date) | ||
|
||
@reservations.each do |booking| | ||
if date.between?(booking.start_date, booking.end_date) | ||
bookings_by_day << booking | ||
end | ||
end | ||
|
||
return bookings_by_day | ||
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,58 @@ | ||
require 'date' | ||
require_relative 'reservation' | ||
|
||
class BlockRoom | ||
|
||
attr_reader :reservations, :blocked_rooms_ids, :check_in, :check_out, :rate | ||
|
||
def initialize(check_in, check_out, rate, blocked_rooms_ids) | ||
if blocked_rooms_ids.count > 5 || blocked_rooms_ids.count < 1 || rate >= 200 | ||
raise ArgumentError.new("Room Blocks must range 1 to 5 rooms, rate must be <200") | ||
end | ||
|
||
@check_in = Date.parse(check_in) | ||
@check_out = Date.parse(check_out) | ||
@rate = rate | ||
@reservations = [] | ||
|
||
# an array of blocked room's ids | ||
@blocked_rooms_ids = blocked_rooms_ids | ||
end | ||
|
||
def add_reservation(reservation) | ||
if @reservations.count > 4 || !@blocked_rooms_ids.include?(reservation.room_id) | ||
raise ArgumentError.new("Room Block") | ||
else | ||
@reservations << reservation | ||
end | ||
end | ||
|
||
# Valid to make this block? | ||
def new_block(check_in, check_out, number_of_rooms, rate) | ||
block_rooms_ids = [] | ||
|
||
rooms_available = available_rooms(check_in, check_out) | ||
|
||
if number_of_rooms <= 5 && rooms_available.count >= number_of_rooms | ||
number_of_rooms.times do | ||
block_rooms_ids << rooms_available[0] | ||
end | ||
|
||
block = BlockRoom.new(check_in, check_out, rate, block_rooms_ids) | ||
|
||
@blocks << block | ||
else | ||
raise ArgumentError.new("Can only block 5 rooms at a time. Can only block available rooms. There are #{available_rooms.count} rooms.") | ||
end | ||
end | ||
|
||
# Modified this method | ||
def reserve_block | ||
new_rez = Reservation.new(@block.check_in.to_s, @block.check_out.to_s, @block.blocked_rooms_ids[0]) | ||
|
||
@block.add_reservation(new_rez) | ||
|
||
return new_rez | ||
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 @@ | ||
require 'date' | ||
|
||
class Reservation | ||
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. I would suggest a method in reservation which either takes a |
||
|
||
attr_reader :end_date, :start_date, :stay, :cost, :room_id | ||
|
||
def initialize(start_date, end_date, room_id) | ||
valid = true | ||
if valid_dates?(start_date, end_date) | ||
@start_date = Date.parse(start_date) | ||
@end_date = Date.parse(end_date) | ||
else | ||
valid = false | ||
end | ||
stay_length if valid | ||
|
||
if !@stay || !valid | ||
raise ArgumentError.new("Invalid date(s)") | ||
end | ||
|
||
@room_id = room_id | ||
end | ||
|
||
def valid_dates?(start_date, end_date) | ||
valid = true | ||
begin | ||
Date.parse(start_date) | ||
Date.parse(end_date) | ||
rescue | ||
valid = false | ||
end | ||
return valid | ||
end | ||
|
||
def stay_cost | ||
@cost = @stay * 200 | ||
end | ||
|
||
def stay_length | ||
@stay = false | ||
date_order = @start_date <=> @end_date | ||
if date_order < 0 | ||
@stay = (@end_date.mjd - @start_date.mjd) | ||
stay_cost | ||
end | ||
end | ||
end |
Oops, something went wrong.
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.
Ummm.. if there were no rooms then
available_rooms
would not includeroom_id
.