Karis - Edges - Hotel

Guardfile

@@ -1,6 +1,6 @@
-guard :minitest, bundler: false, rubygems: false do
+guard :minitest, bundler: false, autorun: false, rubygems: false do
   # with Minitest::Spec
   watch(%r{^spec/(.*)_spec\.rb$})
-  watch(%r{^lib/(.+)\.rb$})         { |m| "spec/#{m[1]}_spec.rb" }
+  watch(%r{^lib/(.+)\.rb$})         { |m| "speguc/#{m[1]}_spec.rb" }
   watch(%r{^spec/spec_helper\.rb$}) { 'spec' }
 end

design-activity.md

@@ -0,0 +1,57 @@
+What classes does each implementation include? Are the lists the same?
+  Both implementation include classes named CartEntry, ShoppingCart, Order. If you mean the class name list by "list" then yes, the list is the same for both implementations.
+
+Write down a sentence to describe each class.
+  For implementation A, class CartEntry stores unit price and quantity. Class ShoppingCart stores all entries in an array. Class Order stores all entries from ShoppingCart then calculates the cost of all entries including tax.
+
+  For implementation B, class CartEntry stores unit price and quantity while also being able to calculate cost of the merchandise with its cost and quantity. Class ShoppingCart stores all entires and can calculate the total cost for all entires. Class Order stores all entries, gets the total price from ShoppingCart then adds tax to calculate total sum.
+
+How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper.
+
+  For implementation A, ShoppingCart is directly instantiated in Order with .new but CartEntry's instance variables are also called in Order. Order is the big class that brings all classes together.
+
+  For implementation B, CartEntry's instance method price, price, is called in ShoppingCart inside it's instance method, price. ShoppingCart's price method is called in Order's instance method, total_price.
+
+What data does each class store? How (if at all) does this differ between the two implementations?
+  For A, CartEntry holds unit_price and quantity, ShoppingCart holds all entires, Order holds all entries and total sum.
+
+  For B, CartEntry holds unit_price, quantity,  price of one merchandise w/o tax,  ShoppingCart holds all entires with price of all merchandise w/o tax, Order holds all entries and total sum with tax.
+
+  Difference is that B holds more data and has more behavior for each of its classes compared to A. A has 2 classes that just holds state.
+
+What methods does each class have? How (if at all) does this differ between the two implementations?
+  A: Only order has an instance method but all classes do have the initialize method. Order#total_price method.
+
+  B: CartEntry#price method, ShoppingCart#price method, Order#total_price method.
+
+  Difference: B has more instance methods in each class so more behavior.
+
+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?
+  A: retained in order
+
+  B: delegated to lower level classes
+
+Does total_price directly manipulate the instance variables of other classes?
+  A: yes, because total_price directly iterates over @cart.entries which is manipulating the instance variable of ShoppingCart.
+
+  B: no, because total_price simply calls the ShoppingCart#price method then assigns it to a local variable then adds the tax to that local variable.
+
+If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify?
+  It would change the unit price to drop if one buys 10 folders in quantity for one order instead of 10 orders with 1 folder. So it could be an if statement of if quantity is multiples of 10 then minus price by 2 dollars.
+
+  B is easier to modify because the if statement can go inside #price method of CartEntry class. For A, I would either need to create an if statement inside the initialize method or create it inside the iteration of the total_price which would make it have too much responsibility in one method.
+
+Which implementation better adheres to the single responsibility principle?
+  B. Because each class has both state and behavior while having one single responsibility.
+
+HOTEL REVISITED:
+
+what changes you would need to make to improve this design, and how the resulting design would be an improvement.
+  BookingSystem takes on the role of checking for overlapping dates which is more than what it should do. Reservation class could instead become a date range class and it could address the overlapping date ranges there. This will allow BookingSystem to have less responsibilities.
+
+  Then I could still have a reservation class where it takes in the confirmed date ranges that can be used to make a reservation and assign a room number to that date range in that class.
+
+  I did not finish wave 2 before when we submitted this project so I focused on completing that part. Then I focused on breaking down the methods with less responsibility for the #make_reservation method.
+
+  The way the methods are set up makes sense for them to be in BookingSystem so I had difficulty with fleshing the responsibilities out in BookingSystem. I am still working on breaking down the booking_system class so I have not committed a repo with a newly improved design but it does have better methods with less responsibility.

hotel-revisited.txt

@@ -0,0 +1,29 @@
+What is this class's responsibility? You should be able to describe it in a single sentence.
+  Reservation: stores check in, check out date, and room number while also calculating the cost for those dates.
+
+  BookingSystem: keeps track of all reservations, assigns room number, checks for overlaps in dates for new booking, list reservations by date and show available rooms.
+
+Is this class responsible for exactly one thing?
+  Reservation: Almost. It's perhaps it will be better if it just focuses on dates rather than also keeping track of the room_number as well as calculating the cost.
+
+  BookingSystem: No. It's checking to see if the dates overlap while trying to confirm the client is able to make a reservation for that date range.
+
+Does this class take on any responsibility that should be delegated to "lower level" classes?
+  Reservation: Not sure if calculating the cost should go to a lower level class but it this class does need to focus on one responsibility.
+
+  BookingSystem: Yes. Overlaps could go to a date range class.
+
+Is there code in other classes that directly manipulates this class's instance variables?
+  Reservation:
+
+  BookingSystem:
+
+How easy is it to follow your own instructions?
+  It was easy for me to follow the instructions because I wrote them today but if I looked at it a week from now then it will be difficult to follow because it's not specific.
+
+Do these refactors improve the clarity of your code?
+  Yes, if implemented it will reduce the number of responsibilities the booking_system class has right now.
+
+
+Do you still agree with your previous assesment, or could your refactor be further improved?
+  I agree with the previous assessment but there are always ways to refactor for one's code to be improved.

lib/.keep

@@ -0,0 +1 @@
+

lib/booking_system.rb

@@ -0,0 +1,60 @@
+require_relative 'reservation'
+
+module Hotel
+  class BookingSystem
+    class AllBookedError < StandardError ; end
+
+    attr_reader :rooms, :reservations
+
+    def initialize
+      @reservations = []
+      @rooms = [*1..20]
+    end
+
+    def make_reservation(check_in_date:, check_out_date:)
+
+      room_number = get_available_room(check_in_date: check_in_date, check_out_date: check_out_date)
+      reservation = Reservation.new(check_in_date: check_in_date, check_out_date: check_out_date, room_number: room_number)
+
+      assign_diff_room_for_overlapping_dates(reservation)
+      @reservations << reservation
+      return reservation
+    end
+
+    def assign_diff_room_for_overlapping_dates(booking)
+      if @reservations.length >= 1
+        booking.date_range.each do |date|
+          if list_reservations_by_date(date).length >= 1
+            booking.room_number = show_available_rooms(date).first
+            raise AllBookedError.new("unable to reserve/rooms all booked") unless !booking.room_number.nil?
+          end
+        end
+      end
+    end
+
+    def get_available_room(check_in_date:, check_out_date:)
+      return @rooms.first
+    end
+
+    def list_all_rooms
+      return @rooms
+    end
+
+    def list_reservations_by_date(date)
+      specific_date = Date.parse("#{date}")
+
+      bookings_by_date = []
+      @reservations.each do |booking|
+        dates = booking.date_range
+        bookings_by_date << booking if dates.include?(specific_date)
+      end
+
+      return bookings_by_date
+    end
+
+    def show_available_rooms(date)
+      show = list_reservations_by_date(date).map {|reservation| reservation.room_number }
+      return list_all_rooms - show
+    end
+  end
+end

lib/reservation.rb

@@ -0,0 +1,28 @@
+require 'date'
+
+module Hotel
+  class Reservation
+    attr_reader :check_in_date, :check_out_date
+    attr_accessor :room_number
+
+    COST = 200
+
+    def initialize(input)
+      @check_in_date = Date.parse("#{input[:check_in_date]}")
+
+      @check_out_date = Date.parse("#{input[:check_out_date]}")
+      raise StandardError, "Checkout date cannot be before checkin date" unless input[:check_in_date] < input[:check_out_date]
+      @room_number = input[:room_number].nil? ? input[:room_number] : 1
+    end
+
+    def date_range
+      date_range = [*check_in_date...check_out_date]
+      return date_range
+    end
+
+    def calculate_booking_cost
+      cost = date_range.length * COST
+      return cost
+    end
+  end
+end

refactors.txt

@@ -0,0 +1,9 @@
+Refactoring ideas:
+
+  BookingSystem might have too many responsibilities so might need to divide it into several classes.
+
+  #make_reservation method has too many responsibilities inside and the if statement is too clumped. Break down the method to several instance methods.
+
+  Use .map instead of .each in #list_reservations_by_date.
+
+  Clean up the tests and organize them so that it's easier to read.

spec/booking_system_spec.rb

@@ -0,0 +1,119 @@
+require_relative 'spec_helper'
+
+describe "BookingSystem class" do
+  before do
+    @booking = Hotel::BookingSystem.new
+    @reservation = @booking.make_reservation(check_in_date: 180904, check_out_date: 180907)
+  end
+
+  it "creates a BookingSystem class" do
+    expect(@booking).must_be_kind_of Hotel::BookingSystem
+  end
+
+  it "returns an array of all instances of reservations created" do
+    expect(@booking.reservations).must_be_kind_of Array
+    expect(@booking.reservations[0]).must_be_kind_of Hotel::Reservation
+  end
+
+  it "returns array of all room numbers in hotel" do
+    expect(@booking.list_all_rooms).must_be_kind_of Array
+    expect(@booking.list_all_rooms).must_equal [*1..20]
+  end
+
+  it "returns array of dates" do
+    expect(@reservation).must_be_kind_of Hotel::Reservation
+    expect(@reservation.date_range).must_equal [Date.parse("180904"), Date.parse("180905"), Date.parse("180906")]
+  end
+
+  it "returns the total cost of the booking" do
+    expect(@reservation.calculate_booking_cost).must_equal 600
+  end
+
+  it "lists all bookings of specific date" do
+    bad_reservation = @booking.make_reservation(check_in_date: 180909, check_out_date: 1809010)
+
+    expect(@booking.list_reservations_by_date(180904)).must_include @reservation
+    expect(@booking.list_reservations_by_date(180904)).wont_include bad_reservation
+  end
+
+  it "assigns different room number if room number is already taken for existing reservation" do
+    @booking.make_reservation(check_in_date: 180904, check_out_date: 180905)
+    @booking.make_reservation(check_in_date: 180904, check_out_date: 180905)
+    fourth_reservation = @booking.make_reservation(check_in_date: 180904, check_out_date: 180905)
+
+    expect(fourth_reservation.room_number).must_equal 4
+    expect(@booking.list_reservations_by_date(180904)).must_include fourth_reservation
+  end
+
+  it "raises argument error when tries to overbook" do
+    19.times do
+      @booking.make_reservation(check_in_date: 180904, check_out_date: 180905)
+    end
+
+    expect{(@booking.make_reservation(check_in_date: 180904, check_out_date: 180905))}.must_raise Hotel::BookingSystem::AllBookedError
+  end
+
+  it "does not raise error when booking for 20th time on same day" do
+    19.times do
+      @booking.make_reservation(check_in_date: 180904, check_out_date: 180905)
+    end
+
+    expect(@booking.list_reservations_by_date(180904).length).must_equal 20
+  end
+
+  it "automatically assigns room number 1 if it\'s the first booking for that date range" do
+    new_reservation = @booking.make_reservation(check_in_date: 180910, check_out_date: 180911)
+# binding.pry
+    expect(new_reservation.room_number).must_equal 1
+    expect(@booking.reservations.length).must_equal 2
+  end
+
+  it "assigns different room number if dates overlap at the front" do
+    new_reservation = @booking.make_reservation(check_in_date: 180903, check_out_date: 180905)
+
+    expect(new_reservation.room_number).must_equal 2
+    expect(@booking.reservations.length).must_equal 2
+  end
+
+  it "assigns different room number if dates overlap at the end" do
+    new_reservation = @booking.make_reservation(check_in_date: 180906, check_out_date: 180908)
+
+    expect(new_reservation.room_number).must_equal 2
+    expect(@booking.reservations.length).must_equal 2
+  end
+
+  it "assigns different room number if dates are contained in existing reservation date_range" do
+    new_reservation = @booking.make_reservation(check_in_date: 180905, check_out_date: 180906)
+
+    expect(new_reservation.room_number).must_equal 2
+    expect(@booking.reservations.length).must_equal 2
+  end
+
+  it "assigns different room number if dates contain the  existing reservation date_range" do
+    new_reservation = @booking.make_reservation(check_in_date: 180903, check_out_date: 180908)
+
+    expect(new_reservation.room_number).must_equal 2
+    expect(@booking.reservations.length).must_equal 2
+  end
+
+  it "creates reservation for checkout date of existing reservation" do
+    new_reservation = @booking.make_reservation(check_in_date: 180907, check_out_date: 180908)
+
+    expect(new_reservation.room_number).must_equal 1
+    expect(@booking.reservations.length).must_equal 2
+  end
+
+  it "creates new reservation with the same checkout date  for the checkin date of existing reservation" do
+    new_reservation = @booking.make_reservation(check_in_date: 180903, check_out_date: 180904)
+
+    expect(new_reservation.room_number).must_equal 1
+    expect(@booking.reservations.length).must_equal 2
+  end
+
+  it "shows all room_numbers that were reserved" do
+    @booking.make_reservation(check_in_date: 180903, check_out_date: 180906)
+
+    expect(@booking.show_available_rooms(180904)).must_be_kind_of Array
+    expect(@booking.show_available_rooms(180904)).must_equal [*3..20]
+  end
+end

spec/reservation_spec.rb

@@ -0,0 +1,28 @@
+require_relative 'spec_helper'
+
+describe "Reservation class" do
+  before do
+    @reservation = Hotel::Reservation.new({check_in_date: 180904, check_out_date: 180905})
+  end
+
+  it "initialize method" do
+    expect(@reservation).must_be_kind_of Hotel::Reservation
+    expect(@reservation.check_in_date).must_be_kind_of Date
+    expect(@reservation.check_out_date).must_be_kind_of Date
+  end
+
+  it "raises ArgumentError if check out date is before check in date" do
+    expect{(Hotel::Reservation.new(180904, 180903))}.must_raise StandardError
+  end
+
+  it "returns all dates in date range" do
+    expect(@reservation.date_range).must_be_kind_of Array
+    expect(@reservation.date_range.length).must_equal 1
+    expect(@reservation.date_range).must_equal [Date.parse("180904")]
+  end
+
+  it "returns cost of that date range" do
+    expect(@reservation.calculate_booking_cost).must_be_kind_of Integer
+    expect(@reservation.calculate_booking_cost).must_equal 200
+  end
+end

spec/spec_helper.rb

@@ -1,8 +1,14 @@
+require 'simplecov'
+SimpleCov.start
+
 require 'minitest'
 require 'minitest/autorun'
 require 'minitest/reporters'
-# Add simplecov
+require 'minitest/skip_dsl'
+require 'awesome_print'
 
 Minitest::Reporters.use! Minitest::Reporters::SpecReporter.new
 
 # Require_relative your lib files here!
+require_relative '../lib/reservation'
+require_relative '../lib/booking_system'