forked from AdaGold/hotel
-
Notifications
You must be signed in to change notification settings - Fork 48
Branches - Monick #30
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
mfunkemomo
wants to merge
27
commits into
Ada-C12:master
Choose a base branch
from
mfunkemomo: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
27 commits
Select commit
Hold shift + click to select a range
991867b
created classes for date_range, reservation, reservation_maker, and room
mfunkemomo de64385
changed reservation_maker to reservation_manager and added book_room …
mfunkemomo 3fe3532
created tests for date_range
mfunkemomo 82ce780
changed date_range class to reservation_dates class. Added initialist…
mfunkemomo 4fd1163
Completed and passed tests for reservation class.
mfunkemomo 722058d
instantiated successully reservation_manager and passed test. very ba…
mfunkemomo 680a740
added rooms method, reservation_list method, and book_room method to …
mfunkemomo d1d985b
created length test for booked rooms and instance test for make_reser…
mfunkemomo 20f0521
updated book_room method test and added test reservation_nights method
mfunkemomo 816c48b
added test for argumenterror for invalid date. added method and test …
mfunkemomo c165c7d
completed test for invalid checkin/out. wave 1 complete. wave 2 in pr…
mfunkemomo c2dee31
corrected make_reservation method by adding parameters. fixed tests a…
mfunkemomo 696d033
updated make_reservation method to save new reservations to reservati…
mfunkemomo 6a10b63
changed book_room method to return hash instead of array and edited m…
mfunkemomo 0e94b5c
still cannot get 2 tests passing but refactored reservation_list and …
mfunkemomo c834473
What not able to make 2 remaining tests pass. Refactored code to 93% …
mfunkemomo 40d82d0
Was not able to pass 2 tests but was able to refactor to 94.87% cover…
mfunkemomo 3445c8b
completed design-activity for revisiting hotel
mfunkemomo 1c9d5c5
Answered questions on revisiting hotel in refactor.txt file
mfunkemomo 6871ba6
delegated methods from reservation_manager to lowerlevel classes. cha…
mfunkemomo 138f448
refactored reservation_dates
mfunkemomo c24c7ce
added notes in reservation for refactoring when loading reservation i…
mfunkemomo 48ee611
combined reservation_list and available_rooms method
mfunkemomo 81bb3aa
refactored reservation_manager and both methods.
mfunkemomo 0593088
began testing, tests for ReservationDate class passed
mfunkemomo d39890e
refactored and passed tests for reservation class
mfunkemomo be810b2
in the end, i moved back some methods to reservation_manager because …
mfunkemomo 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 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,45 @@ | ||
| 1. What classes does each implementation include? Are the lists the same? | ||
| - Implementation A and B both have the same number of classes and the same classes: | ||
| - CartEntry | ||
| - ShoppingCart | ||
| - Order | ||
|
|
||
| 2. Write down a sentence to describe each class. | ||
| - CartEntry: is a class that takes in an item that takes in a unit price and quantity that gets put in the cart. | ||
| - ShoppingCart: is a class that contains an array of items and its quantities | ||
| - Order: is a class that takes in the entries and calculates the total cost of those entries. | ||
|
|
||
| 3. How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper. | ||
| - It looks like Order takes in or is composed of ShoppingCart and ShoppingCart is composed of CartEntry. | ||
|
|
||
| 4. What data does each class store? How (if at all) does this differ between the two implementations? | ||
| - CartEntry: is integers for the unit price and quantities. | ||
| - ShoppingCart: is an array of the unit price and quantities. | ||
| - Order: takes in a class of ShoppingCart which accesses the unit prices and quantities and returns a total price (integer) for everything in the ShoppingCart instance. | ||
| - In Implementation A, it seems like the first two classes are just meant for storing and the actual methods are being done in Order but in Implementation B each class has a method that calculates its relevant total prices so that not all the calculation is done in Order. | ||
|
|
||
| 5. What methods does each class have? How (if at all) does this differ between the two implementations? | ||
| - CartEntry: Both implementations instantiates but Implementation B includes a method that calculates the price for the entry and its quantities. | ||
| - ShoppingCart: Both implementations instantiates but Implementation B includes a method that calculates the price for the entire array of entries. | ||
| - Order: Both implementations instantiates but Implementation B's method has been broken down in the other classes so now it simply takes the total from ShoppingCart and adds tax to get a true total price. | ||
|
|
||
| 6. Consider the Order#total_price method. In each implementation: | ||
| a. Is logic to compute the price delegated to "lower level" classes like ShoppingCart and CartEntry, or is it retained in Order? | ||
| - Implementation A retains and does all the computation in Order. | ||
| - Implementation B delegates lower level pricing calculation to the other classes so that Order does not do all of it. | ||
| b. Does total_price directly manipulate the instance variables of other classes? | ||
| - Implementation A accesses the instance variable of ShoppingCart and CartEntry. | ||
| - Implementation B accesses the instance variable of ShoppingCart and ShoppingCart method. | ||
|
|
||
| 7. If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify? | ||
| - Since CartEntry takes in the item that is being adding to ShoppingCart, I think it would make sense to implement this in CartEntry. If quantity = :bulk, there can be a method in CartEntry that calculates the discounted price for the bulk item. | ||
|
|
||
| 8. Which implementation better adheres to the single responsibility principle? | ||
| - Implementation B better adheres to the single responsibility principle as it's single behavior and state. | ||
|
|
||
| 9. Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled? | ||
| - EntryCart and ShoppingCart are loosely coupled as ShoppingCart relies on what EntryCart puts into ShoppingCart. | ||
|
|
||
| FOR MY HOTEL PROJECT | ||
| - will need to delegate methods to other low level classes | ||
| - break up long methods into smaller methods |
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,22 @@ | ||
| require_relative 'reservation_dates' | ||
|
|
||
| module HotelBookings | ||
| class Reservation | ||
| def initialize(customer_name:, checkin:, checkout:, room_no:) | ||
| @customer_name = customer_name | ||
| @checkin = checkin | ||
| @checkout = checkout | ||
| @room_no = room_no | ||
|
|
||
| end | ||
|
|
||
| #would like to have self.all and load all reservations made in running memory. Should return a hash | ||
|
|
||
| def total_cost | ||
| dates = ReservationDates.new(checkin:@checkin, checkout:@checkout) | ||
| subtotal = dates.total_nights * 200 | ||
| total = subtotal * 1.101 | ||
| return total.round(2) | ||
| 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,22 @@ | ||
| require 'date' | ||
|
|
||
| module HotelBookings | ||
| class ReservationDates | ||
| attr_reader :checkin, :checkout | ||
|
|
||
| def initialize(checkin:, checkout:) | ||
| @checkin = Date.parse(checkin) | ||
| @checkout = Date.parse(checkout) | ||
| end | ||
|
|
||
| def date_range | ||
| date_range = @checkin...@checkout | ||
| dates_array = date_range.to_a | ||
| return dates_array | ||
| end | ||
|
|
||
| def total_nights | ||
| return date_range.length | ||
| 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,77 @@ | ||
| require 'date' | ||
| require_relative 'reservation_dates' | ||
| require_relative 'reservation' | ||
|
|
||
| module HotelBookings | ||
| class Reservation_Manager | ||
| MAX_ROOMS = 20 | ||
| attr_reader :customer_name, :checkin, :checkout | ||
|
|
||
| def initialize(customer_name:, checkin:, checkout:) | ||
| @customer_name = customer_name | ||
| @checkin = checkin | ||
| @checkout = checkout | ||
|
|
||
| #would like to remove this and load all reservation classes instead. Not sure how... | ||
| @current_reservations = {1 => [], 2 => [], 3 => [], 4 => [], 5 => [], 6 => [], 7 => [], 8 => [], 9 => [], 10 => [], 11 => [], 12 => [], 13 => [], 14 => [], 15 => [], 16 => [], 17 => [], 18 => [], 19 => [], 20 => []} | ||
| end | ||
|
|
||
| def rooms | ||
| return @current_reservations | ||
| end | ||
|
|
||
| def rm_availibility_list(date, list_type) | ||
| valid_list_type = %i[available_rooms unavailable_rooms] | ||
| if valid_list_type.include?(list_type) == false | ||
| raise ArgumentError.new "Invalid list type." | ||
| end | ||
|
|
||
| unavailable_rooms = [] | ||
| available_rooms = [] | ||
| @current_reservations.each do |room| | ||
| if room.include?(date) == true | ||
| unavailable_rooms.push(room) | ||
| else | ||
| available_rooms.push(room) | ||
| end | ||
| end | ||
|
|
||
| case list_type | ||
| when :unavailable_rooms | ||
| return unavailable_rooms.uniq | ||
| when :available_rooms | ||
| return available_rooms.uniq | ||
| end | ||
| end | ||
|
|
||
| def reserve_rooms | ||
| booked_rooms = {} | ||
| new_res = ReservationDates.new(checkin:@checkin, checkout:@checkout) | ||
| new_res.date_range.each do |night| | ||
| nights_booked = [] | ||
| available_rooms = rm_availibility_list(night, :available_rooms) | ||
| if available_rooms.length > 0 && booked_rooms.empty? == true | ||
| nights_booked.push(night) | ||
| booked_rooms[available_rooms.first] = nights_booked | ||
| elsif available_rooms.length > 0 && booked_rooms.empty? == false | ||
| if booked_rooms.include?(available_rooms.first) == true | ||
| booked_rooms[available_rooms.first] = nights_booked.push(night) | ||
| end | ||
| else | ||
| raise ArgumentError.new("No rooms available for #{night}") | ||
| end | ||
| end | ||
| return booked_rooms | ||
| end | ||
|
|
||
| def make_reservation | ||
| if @checkout < @checkin | ||
| raise ArgumentError.new('Invalid checkin/checkout date') | ||
| end | ||
|
|
||
| new_reservation = Reservation.new(customer_name:@customer_name, checkin: @checkin, checkout: @checkout, room_no: reserve_rooms) | ||
|
|
||
| return new_reservation | ||
| 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,34 @@ | ||
| - change the way current_reservations saves | ||
| - maybe load all Reservation instances | ||
| - change tests to pass saving newly made reservations | ||
| - add Booking class for wave 3 | ||
| - check methods that check for unavailable/available rooms | ||
|
|
||
| ========================================== | ||
|
|
||
| Revisiting Hotel | ||
|
|
||
| 1. What is this class's responsibility? You should be able to describe it in a single sentence. | ||
| - ReservationDates: is responsible for changing the string dates to a date class and calculate the total nights in a reservation. | ||
| - ReservationManager: is responsible for many things: keeping a record of all current reservations, creating a reservation list of all current reservations, creating a list of all current available rooms, finding the nights a reservation was made for, booking or keep rooms on hold, and making a reservation. | ||
| - Reservation: is responsibility for creating reservation instances and calculating the total cost of that reservation instance. | ||
|
|
||
| 2. Is this class responsible for exactly one thing? | ||
| - ReservationDates: Yes, it's responsible for the dates/nights for a reservation. | ||
| - ReservationManager: No, it's responsible for many different aspects of making a reservation. | ||
| - Reservation: Yes, it's responsible for the reservation itself and its cost. | ||
|
|
||
| 3. Does this class take on any responsibility that should be delegated to "lower level" classes? | ||
| - ReservationManager definitely does. For things like the total nights and the available rooms and current reservations should have been delegated to lower level classes. | ||
|
|
||
| 4. Is there code in other classes that directly manipulates this class's instance variables? | ||
| - ReservationManager dabbles in all the classes directly as of right now. The other two classes do not do touch other class properties. | ||
|
|
||
| 5. How easy is it to follow your own instructions? | ||
| - While the instructions themselves may be simple but I know the work to get it going will take awhile. | ||
|
|
||
| 6. Do these refactors improve the clarity of your code? | ||
| - While I think they are helpful they do not address much about single responsibility and therefore may not help when clarifying code. | ||
|
|
||
| 7. Do you still agree with your previous assessment, or could your refactor be further improved? | ||
| - Definitely room for lots of improvement. |
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,32 @@ | ||
| require_relative 'test_helper' | ||
|
|
||
| describe "ReservationDates class" do | ||
| before do | ||
| @dates = HotelBookings::ReservationDates.new(checkin:'2019-09-01', checkout:'2019-09-03') | ||
| end | ||
|
|
||
| it "creates an instance of ReservationDates" do | ||
| expect(@dates).must_be_kind_of HotelBookings::ReservationDates | ||
| end | ||
|
|
||
| it "make checkin and check out as Date classes" do | ||
| expect(@dates.checkin).must_be_kind_of Date | ||
| expect(@dates.checkout).must_be_kind_of Date | ||
| end | ||
|
|
||
| it "check if total_nights calculates correctly" do | ||
| expect(@dates.total_nights).must_equal 2 | ||
| end | ||
|
|
||
| it "check if total_nights is an integer" do | ||
| expect(@dates.total_nights).must_be_kind_of Integer | ||
| end | ||
|
|
||
| it "date_range returns a list of nights for the date range" do | ||
| new_res = @dates.date_range | ||
| expect(new_res.length).must_equal 2 | ||
| expect(new_res).must_be_kind_of Array | ||
| expect(new_res[1]).must_be_kind_of Date | ||
| 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,79 @@ | ||
| require_relative 'test_helper' | ||
|
|
||
| describe "Reservation_Manager class" do | ||
| before do | ||
| @rm_test = HotelBookings::Reservation_Manager.new(customer_name: 'Ted', checkin: '2019-09-01', checkout: '2019-09-03') | ||
| end | ||
|
|
||
| describe "instantiation" do | ||
| it "creates an instance of reservation_manager" do | ||
| expect(@rm_test).must_be_kind_of HotelBookings::Reservation_Manager | ||
| end | ||
| end | ||
|
|
||
| describe "list of all rooms/current reservations" do | ||
| it "rooms method displays a list(array) of 20 rooms" do | ||
| expect(@rm_test.rooms.keys).must_be_kind_of Array | ||
| expect(@rm_test.rooms.length).must_equal 20 | ||
| end | ||
| end | ||
|
|
||
| describe "availbility of rooms list" do | ||
| it "rm_availability_list returns list of unavailable rooms and available rooms based on symbol parameter for a given date" do | ||
| ur_list = @rm_test.rm_availibility_list('2019-09-01', :unavailable_rooms) | ||
|
|
||
| ar_list = @rm_test.rm_availibility_list('2019-09-01', :available_rooms) | ||
|
|
||
| expect(ur_list).must_be_kind_of Array | ||
| expect(ar_list).must_be_kind_of Array | ||
| end | ||
|
|
||
| # it "unavailable and available lists do not have any overlapping rooms" do | ||
|
|
||
| # end | ||
| end | ||
|
|
||
| describe "reserve_rooms method, holds/bookes room" do | ||
| it "find an available room for each night of reservation_dates" do | ||
| new_res = @rm_test.reserve_rooms | ||
|
|
||
| expect(new_res.length).must_equal 1 | ||
| expect(new_res).must_be_kind_of Hash | ||
| expect(new_res[1]).must_be_kind_of Array | ||
| end | ||
|
|
||
| # it "should raise an Argument Error if no rooms are available" do | ||
| # @new_res = HotelBookings::Reservation_Manager.new(customer_name: 'Momo', checkin:'2019-09-01', checkout:'2019-09-02') | ||
| # 20.times do | ||
| # @new_res.make_reservation | ||
| # end | ||
| # expect{ | ||
| # @bookingtest.make_reservation | ||
| # }.must_raise ArgumentError | ||
| # end | ||
| end | ||
|
|
||
| # describe "make_reservation method, makes the reservation" do | ||
| # before do | ||
| # new_res = @bookingtest.make_reservation | ||
| # end | ||
|
|
||
| # it "make_reservation method makes a reservation (instance)" do | ||
| # expect(new_res).must_be_kind_of HotelBookings::Reservation | ||
| # end | ||
|
|
||
| # it "raises argument error if checkout/checkin date are invalid" do | ||
| # new_rm = HotelBookings::Reservation_Manager.new(customer_name: 'Momo', checkin:'2019-09-03', checkout:'2019-09-01') | ||
|
|
||
| # expect{ | ||
| # new_rm.make_reservation | ||
| # }.must_raise ArgumentError | ||
| # end | ||
|
|
||
| # it "new reservation is saved in current_reservations" do | ||
| # expect{ | ||
| # @current_reservations.has_value?([Date.parse('2019-09-01'), Date.parse('2019-09-02')]) | ||
| # }.must_equal true | ||
| # 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,19 @@ | ||
| require_relative 'test_helper' | ||
|
|
||
| describe "Reservation class" do | ||
| before do | ||
| @reservation = HotelBookings::Reservation.new(customer_name: 'Ted', checkin:'2019-09-01', checkout:'2019-09-03', room_no: 18) | ||
| end | ||
|
|
||
| it "creates an instance of Reservation" do | ||
| expect(@reservation).must_be_kind_of HotelBookings::Reservation | ||
| end | ||
|
|
||
| it "total_cost method correctly calculates total cost of reservation" do | ||
| expect(@reservation.total_cost).must_equal 440.40 | ||
| end | ||
|
|
||
| it "total_cost method checks if total_cost rounds 2 decimals" do | ||
| expect(@reservation.total_cost).must_be_kind_of Float | ||
| 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 |
|---|---|---|
| @@ -1,8 +1,14 @@ | ||
| # Add simplecov | ||
| require 'simplecov' | ||
| SimpleCov.start | ||
| 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/reservation_dates' | ||
| require_relative '../lib/reservation' | ||
| require_relative '../lib/reservation_manager' |
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.
Nitpick: the Ruby naming convention would call this
ReservationManager, without the_