forked from AdaGold/hotel
-
Notifications
You must be signed in to change notification settings - Fork 48
branches - vi #27
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
Open
criacuervos
wants to merge
14
commits into
Ada-C12:master
Choose a base branch
from
criacuervos: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 - vi #27
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
3f8312a
created the classes and instantiated reservation object
criacuervos 7a1face
Initialized date_range class, as well as added tests to verify its in…
criacuervos 9107567
added to the date range, reservation, and reservation_maker tests and…
criacuervos 5f46a08
added simplecov to the mix!
criacuervos a9b6862
Added reservations_lookup for given date code & test, as well as incl…
criacuervos 6b01fa9
added tests and now on wave 2
criacuervos 3dbcb01
created date overlap method and first test about equivalent date ranges
criacuervos 848af5f
created an overlap? method in date_range class, along with tests and …
criacuervos 0e071ab
updated reserve_room method and added available rooms method along wi…
criacuervos d35c8fb
fixed overlap test issues- all tests now passing
criacuervos a44dbc1
updated block class, added tests, and added reserve room in block fun…
criacuervos af89d34
added test to raise error if reserving room in block for wrong date r…
criacuervos f8f7adc
finished prompt questions
criacuervos 41cfd5b
finished break week homework assignment
criacuervos 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 hidden or 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,24 @@ | ||
| require 'date' | ||
| class ReservationError < StandardError | ||
| end | ||
|
|
||
| module Hotel | ||
| class Block | ||
| attr_accessor :date_range | ||
|
|
||
| def initialize(check_in, check_out, block_rooms = nil, discount = nil) | ||
|
|
||
| @check_in = check_in | ||
| @check_out = check_out | ||
|
|
||
| if block_rooms | ||
| if block_rooms.length > 5 || block_rooms.length < 1 | ||
| raise ReservationError, "You can only have up to 5 rooms in a block" | ||
| end | ||
| end | ||
|
|
||
| total_nights = (@check_out) - (@check_in) - 1 | ||
| @discount = total_nights * 150 | ||
| end | ||
| end | ||
| end |
This file contains hidden or 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,38 @@ | ||
| require_relative 'reservation' | ||
| require_relative 'reservation_maker' | ||
|
|
||
| module Hotel | ||
| class Date_Range | ||
| attr_accessor :check_in, :check_out | ||
|
|
||
| def initialize(check_in, check_out) | ||
| @check_in = check_in | ||
| @check_out = check_out | ||
|
|
||
| if check_out != nil | ||
| if @check_out <= @check_in | ||
| raise ArgumentError, "Invalid date range!" | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def date_overlap?(other) | ||
| return (check_in...check_out).cover?(other.check_in) || | ||
| (other.check_in...other.check_out).cover?(check_in) || | ||
| (other.check_in...other.check_out) == (check_in...check_out) | ||
| end | ||
|
|
||
| def include?(date) | ||
| if date >= @check_in && date < @check_out | ||
| return true | ||
| else | ||
| return false | ||
| end | ||
| end | ||
|
|
||
| def nights | ||
| total_nights = check_out - check_in | ||
| return total_nights | ||
| end | ||
| end | ||
| end | ||
This file contains hidden or 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,69 @@ | ||
| 1) What classes does each implementation include? Are the lists the same? | ||
| In Implementation A & B, they both have classes for ClassEntry, ShoppingCart, and Order. However, the lists are not the same. Inside of the classes, each implementation has a few differences in which methods it uses in which class. | ||
|
|
||
| 2) Write down a sentence to describe each class. | ||
| -The CartEntry class is responsible for creating CartEntry objects which know a unit's price and quantity. | ||
| -The ShoppingCart is in charge of knowing a list of all of the entries of a consumer's cart. | ||
| -In the Order class, the price of all of the entries is calculated to a sum that accounts for sales tax. | ||
|
|
||
| 3) How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper. | ||
| The ShoppingCart class contains many instances of CartEntry, all contained inside of an array, the @entries instance variable. | ||
| Then, the Order class consists of one instance of the ShoppingCart class, which is seen in the @cart instance variable. | ||
|
|
||
| 4) What data does each class store? How (if at all) does this differ between the two implementations? | ||
| In CartEntry, each instance stores that unit's price and quantity. In Implementation B, it also stores the price if that method is called. | ||
|
|
||
| For ShoppingCart, it holds CartEntry objects. Implementation B again does a price method, which when called, will know the total sum of all of the entry's prices. | ||
|
|
||
| For Order, it stores information for the price of Sales Tax, as well as an instance of ShoppingCart. Implementation A calculates the total sum here by looping through each entry in the ShoppingCart entries array. In Implementation B, Order calls the sum method already placed in ShoppingCart and adds sales tax to it for the subtotal. | ||
|
|
||
| 5) What methods does each class have? How (if at all) does this differ between the two implementations? | ||
| -Implementation A only has initialization methods in each class except for in the Order class, which calculates the sum with sales tax added. | ||
| -Implementation B has a method to find the price in each class, whether it's the individual CartEntry price or the sum of the ShoppingCart. | ||
|
|
||
| 6) 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: | ||
| -The only place where price calculations are happening are in the Order class. | ||
| -The total_price method is using the @entries instance variable from cart, and each entry's unit price and quantity. It is using them in the loop and then in multiplication. | ||
| ~In Implementation B: | ||
| -The logic to compute price is delegated to lower level classes. It is calling the price method in ShoppingCart to calculate the sum and then add the tax. | ||
| -In this implementation it does not manipulate the instance variables of the other classes. | ||
|
|
||
| 7) If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify? | ||
| I think Implementation A is easier to modify. If the quantity of items affects the price, then once you are doing price calculations in order, you can set it up in that class to modify it based of the quantity. | ||
| In Implementation B, I think that it would be more complex. As it is currently set up (without redesigning responsibility), you would have to alter the price method in ShoppingCart based on the quantity which just seems unnecessary and not its responsibility. | ||
|
|
||
| 8) Which implementation better adheres to the single responsibility principle? | ||
| I'd argue Implementation A is better because it seems clear to me what each class is doing, and what it exists for. Some classes exist to store data, some exist to store lists of that data, and finally there is a class which does the computation. | ||
|
|
||
| 9) Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled? | ||
| Both implementations are creating a new instance of ShoppingCart in the Order class. There is some coupling which exists in both. Implementation A is more loosely coupled. It is only calling on ShoppingCart so that it can iterate through all of the entries, and find the sum. However, in Implementation B, the total_price method is calling a whole method from ShoppingCart as well. Then, in that price method, it is calling CartEntry's price method. The dependency there runs more deep. | ||
|
|
||
|
|
||
| ------------------------------------------------ | ||
|
|
||
|
|
||
| Responsibility of each class: | ||
| -Date_Range is responsible for creating objects that know a check in and check out date, and ensuring that two dates don't overlap. | ||
| -Reservation class is in charge of knowing its date range, room number, and block. | ||
| -Block class is in charge of knowing its date range, and the room numbers it consists of, and cost. | ||
| -ReservationMaker is in charge of handling reservation and block making, and storing a list of available rooms. | ||
|
|
||
|
|
||
| Describe what changes you would need to make to improve this design, and how the resulting design would be an improvement. | ||
|
|
||
| For this activity, I worked on updating my Reservation and Block classes' instance methods. It takes an object of date range, which is fine for me to understand and use, but means that it is tightly coupled. and that to use and understand it in tests might be abstract for other people. In order to test the Reservation & Block class however, you have to initialize an object of Date Range as well. | ||
|
|
||
| I figured it would be easiest and best practice for anybody using my code to not have to do that. So I updated Reservation & Block to take in a check in and check out date as a unix timestamp rather than depending on creating a date range object. | ||
|
|
||
|
|
||
| Something else I refactored was the constructor in the Block class so that it would not need an instance of ReservationMaker. I used that to find and assign available rooms for the Block object. I decided that it isn't the responsibility of that class to know which rooms were available, but just a max limit in how many rooms in can have. | ||
|
|
||
| I also updated the reserve_from_block method in the ReservationMaker class. I added a helper method (block_lookup) that would search through blocks, and is then utilized in reserve_from_block. | ||
|
|
||
| There is definitely still more I could have done, but I see pretty clearly in what ways loosening up coupling really helps make the code more universal and therefore easily modifiable in future situations. | ||
|
|
||
|
|
||
|
|
This file contains hidden or 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,26 @@ | ||
| require 'date' | ||
|
|
||
| module Hotel | ||
| class Reservation | ||
| attr_reader :check_in, :check_out, :room | ||
| attr_accessor :date_range | ||
|
|
||
| def initialize(check_in, check_out, room = nil, block_reference = nil ) | ||
| @check_in = check_in | ||
| @check_out = check_out | ||
|
|
||
| if room | ||
| @room = room | ||
| end | ||
| if block_reference | ||
| @block_reference = block_reference | ||
| end | ||
| end | ||
|
|
||
| def cost | ||
| total_stay = @check_out - @check_in - 1 | ||
| cost = total_stay * 200 | ||
| return cost | ||
| end | ||
| end | ||
| end |
This file contains hidden or 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,96 @@ | ||
| require_relative 'reservation' | ||
| require_relative 'date_range' | ||
| require_relative 'block' | ||
| class ReservationError < StandardError | ||
| end | ||
|
|
||
| module Hotel | ||
| class Reservation_Maker | ||
| attr_reader :rooms | ||
| attr_accessor :reservations, :blocks | ||
|
|
||
| def initialize | ||
| rooms = Array(1..20) | ||
| @rooms = rooms | ||
| @reservations = [] | ||
| @blocks = [] | ||
| end | ||
|
|
||
| def reserve_from_block(check_in, check_out) | ||
| date_range = Hotel::Date_Range.new(check_in, check_out) | ||
| block = block_lookup(date_range) | ||
| rooms = available_rooms(date_range) | ||
| room = rooms[0] | ||
|
|
||
| if room == nil | ||
| raise ReservationError.new("There are no available rooms for the chosen block") | ||
| end | ||
|
|
||
| block_reference = Hotel::Block.new(check_in, check_out) | ||
| block_reservation = Hotel::Reservation.new(check_in, check_out, room, block_reference) | ||
| add_reservation(block_reservation) | ||
| return block_reservation | ||
| end | ||
|
|
||
| def block_lookup(date_range) | ||
| @blocks.find { |block| block.date_range == date_range } | ||
| end | ||
|
|
||
| def reserve_room(check_in, check_out) | ||
| date_range = Hotel::Date_Range.new(check_in, check_out) | ||
|
|
||
| @reservations.each do |reservation| | ||
| check_in = reservation.check_in | ||
| check_out = reservation.check_out | ||
| other = Hotel::Date_Range.new(check_in, check_out) | ||
| if date_range.date_overlap?(other) | ||
| raise ReservationError.new("Reservation overlaps with existing reservation") | ||
| end | ||
| end | ||
| avail_rooms = available_rooms(date_range) | ||
| available_room = avail_rooms[0] | ||
|
|
||
| reservation = Hotel::Reservation.new(date_range, available_room) | ||
| add_reservation(reservation) | ||
| return reservation | ||
| end | ||
|
|
||
| def add_reservation(reservation) | ||
| reservations << reservation | ||
| return reservations | ||
| end | ||
|
|
||
| def add_block(block) | ||
| blocks << block | ||
| return blocks | ||
| end | ||
|
|
||
| def reservations_lookup(date) | ||
| all_reservations_by_date = [] | ||
| @reservations.each do |reservation| | ||
| if reservation.date_range.include?(date) | ||
| all_reservations_by_date << reservation | ||
| end | ||
| end | ||
| return all_reservations_by_date | ||
| end | ||
|
|
||
| def available_rooms(date_range) | ||
| unavail_rooms = [] | ||
|
|
||
| reservations.each do |reservation| | ||
| if reservation.date_range.date_overlap?(date_range) | ||
| unavail_rooms << reservation.room | ||
| end | ||
| end | ||
|
|
||
| avail_rooms = [] | ||
| rooms.each do |room| | ||
| unless unavail_rooms.include?(room) | ||
| avail_rooms << room | ||
| end | ||
| end | ||
| return avail_rooms | ||
| end | ||
| end | ||
| end |
This file contains hidden or 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 @@ | ||
| require_relative 'test_helper' | ||
|
|
||
| describe "Block class" do | ||
| before do | ||
| @reservation_maker = Hotel::Reservation_Maker.new | ||
| @check_in = Date.new(2020, 01, 01) | ||
| @check_out = @check_in + 4 | ||
| @block_rooms = @reservation_maker.rooms[0..3] | ||
| @discount = 150 | ||
|
|
||
| @block = Hotel::Block.new(@check_in, @check_out, @block_rooms, @discount) | ||
| @room = @block_rooms[0] | ||
| end | ||
|
|
||
| describe "initialize" do | ||
| it "is an instance of Block" do | ||
| expect(@block).must_be_kind_of Hotel::Block | ||
| end | ||
|
|
||
| it "raises an error if room count is greater than max of 5" do | ||
| block_rooms = @reservation_maker.rooms[0..20] | ||
| expect{ Hotel::Block.new(@check_in, @check_out, block_rooms) }.must_raise ReservationError | ||
| 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.
Clever and concise!