Val - Edges - Hotel

.gitignore

@@ -48,3 +48,4 @@ build-iPhoneSimulator/
 
 # unless supporting rvm < 1.11.0 or doing something fancy, ignore this:
 .rvmrc
+coverage

Guardfile

@@ -1,4 +1,4 @@
-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" }

design-activity.md

@@ -0,0 +1,53 @@
+What classes does each implementation include? Are the lists the same?
+  Both implementations have the same classes - CartEntry, ShoppingCart, and Order.
+
+Write down a sentence to describe each class.
+  CartEntry creates entries (items to be purchased).
+  ShoppingCart keeps track of a collection of entries.
+  Order calculates the total price of a ShoppingCart.
+
+How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper.
+  An Order has one ShoppingCart.
+  A ShoppingCart has many CartEntries.
+
+What data does each class store? How (if at all) does this differ between the two implementations?
+  Both implementations store the same data.
+  CartEntry stores unit_price and quantity in instance variables.
+  ShoppingCart stores instances of CartEntry in an array.
+  Order stores SALES_TAX in a constant.
+
+What methods does each class have? How (if at all) does this differ between the two implementations?
+  Implementation A:
+    CartEntry has a unit_price and quantity method.
+    ShoppingCart has an entries method.
+    Order has a total_price method.
+  Implementation B:
+    CartEntry and ShoppingCart have price methods.
+    Order has a total_price method.
+  All classes in both implementations have constructor methods.
+
+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:
+    All logic to compute the price is retained in Order.
+  Implementation B:
+    Logic to compute the price is delegated to the lower level classes CartEntry and ShoppingCart in the price methods.
+
+Does total_price directly manipulate the instance variables of other classes?
+  Implementation A:
+    Order#total_price directly manipulates the entries instance variable from ShoppingCart by iterating over the array.
+  Implementation B:
+    This implementation doesn't directly manipulate instance variables of other classes and instead uses the price instance methods to achieve the same result as implementation A.
+
+If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify?
+  Adding a discount for items bought in bulk could be achieved by storing additional data in the CartEntry class - perhaps a hash with quantities as keys and discounts as values.
+  I think it would be easier to modify implementation B; you could add the hash to CartEntry and wouldn't need to change anything else. Since the price calculation is done in the CartEntry class (not in the Order class as in implementation A), this shouldn't cause unexpected changes in other classes.
+
+Which implementation better adheres to the single responsibility principle?
+  I think implementation B better adheres to SRP, because each class calculates it's own price.
+
+Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled?
+  I struggled with tight coupling in Hotel, but I think implementation B is more loosely coupled, because it doesn't directly manipulate the instance variables of other classes. Only the public price methods are used to send messages between objects, rather than the instance variables/methods in implementation A.
+
+Improve Hotel design:
+  There are many changes I'd like to make, and I added more notes to refactor.txt based on the instructor feedback. However, I think the most significant item to focus on is the tight coupling where BookingManager directly manipulates instance variables in Calendar and Block. To do this, I'll remove these lines of code from BookingManager and move them to new methods in Calendar and Block, then call those methods from BookingManager. Removing these unnecessary dependencies will achieve looser coupling, and the application will be easier to change in the future.

lib/block.rb

@@ -0,0 +1,37 @@
+require_relative 'reservation'
+
+module Hotel
+  class Block < Reservation
+
+    attr_reader :number_of_rooms, :cost, :rooms
+
+    def initialize(check_in, check_out, number_of_rooms)
+      super(check_in, check_out)
+
+      @number_of_rooms = number_of_rooms
+      if number_of_rooms < 2 || number_of_rooms > 5
+        raise ArgumentError, "Number of rooms must be between 2 and 5"
+      end
+
+      size_discounts = {
+        2 => 0.05 * PRICE,
+        3 => 0.1 * PRICE,
+        4 => 0.15 * PRICE,
+        5 => 0.2 * PRICE
+      }
+
+      @cost -= size_discounts[number_of_rooms]
+
+      @rooms = {}
+    end
+
+    def set_available_status(room)
+      rooms[room] = :available
+    end
+
+    def set_unavailable_status(room)
+      rooms[room] = :unavailable
+    end
+
+  end
+end

lib/booking_manager.rb

@@ -0,0 +1,42 @@
+module Hotel
+  class BookingManager
+    class NoAvailabilityError < StandardError ; end
+
+    attr_reader :calendar, :reservations, :blocks
+
+    def initialize(calendar)
+      @calendar = calendar
+      @reservations = []
+      @blocks = []
+    end
+
+    def reserve_room(check_in, check_out)
+      reservation = Hotel::Reservation.new(check_in, check_out)
+      room = calendar.add_reservation(reservation)
+      @reservations << [reservation, room]
+    end
+
+    def reserve_block(check_in, check_out, number_of_rooms)
+      block = Hotel::Block.new(check_in, check_out, number_of_rooms)
+      rooms = calendar.add_block(block)
+      @blocks << block
+    end
+
+    def reserve_block_room(block)
+      available = block.rooms.select do |room, status|
+        status == :available
+      end
+
+      if available.empty?
+        raise NoAvailabilityError.new("No availability.")
+      end
+
+      reserved_room = available.first[0]
+
+      block.set_unavailable_status(reserved_room)
+
+      return reserved_room
+    end
+
+  end
+end

lib/calendar.rb

@@ -0,0 +1,60 @@
+module Hotel
+  class Calendar
+
+    attr_reader :rooms, :room_assignments
+
+    def initialize(number_of_rooms)
+      @rooms = [*1..(number_of_rooms)]
+      @room_assignments = Hash[ @rooms.collect { |room| [room, []] } ]
+    end
+
+    def add_reservation(reservation)
+      room = available_rooms(reservation).first
+
+      raise StandardError, "No availability." if room.nil?
+
+      reservation.get_all_dates.each do |date|
+        room_assignments[room] << date
+      end
+
+      return room
+    end
+
+    def add_block(block)
+      available_rooms = available_rooms(block)
+      block_size = block.number_of_rooms
+
+      if available_rooms.length < block_size
+        raise StandardError, "No availability."
+      end
+
+      block_rooms = available_rooms.first(block_size)
+
+      block_rooms.each do |room|
+        room_assignments[room] << block.get_all_dates
+        block.set_available_status(room)
+      end
+
+      return block_rooms
+    end
+
+    def reservations(date)
+      date = Date.parse(date)
+      reservations = room_assignments.select { |room, dates|  dates.flatten.include? (date) }.keys
+    end
+
+    def available_room?(room, reservation)
+      dates = reservation.get_all_dates
+      if dates.any? { |date| room_assignments[room].flatten.include? (date)}
+        return false
+      end
+
+      return true
+    end
+
+    def available_rooms(reservation)
+      available_rooms = rooms.select { |room| available_room?(room, reservation)}
+    end
+
+  end
+end

lib/reservation.rb

@@ -0,0 +1,48 @@
+require 'date'
+module Hotel
+  class Reservation
+    class InvalidDateError < StandardError ; end
+
+    PRICE = 200
+
+    attr_reader :check_in, :check_out, :cost
+
+    def initialize(check_in, check_out)
+      @check_in = date_format(check_in)
+      @check_out = date_format(check_out)
+
+      if number_of_nights <= 0
+        raise Hotel::Reservation::InvalidDateError.new("Invalid date range.")
+      end
+
+      @cost = PRICE * number_of_nights
+    end
+
+    def date_format(date)
+      pattern = /^\d{6}$/
+
+      if pattern.match(date).nil?
+        raise ArgumentError.new("Date format: YYMMDD.")
+      else
+        Date.parse(date)
+      end
+    end
+
+    def number_of_nights
+      nights = check_out - check_in
+      return nights.numerator
+    end
+
+    def get_all_dates
+      all_dates = []
+      date = check_in
+
+      number_of_nights.times do
+        all_dates << date
+        date += 1
+      end
+
+      return all_dates
+    end
+  end
+end

refactor.txt

@@ -0,0 +1,39 @@
+Notes from original submission - 9.9.18:
+
+BookingManager methods need to be refactored. A lot of logic is repeated, but I had trouble writing the methods so that both blocks and reservations can use them.
+
+Wrap classes in Hotel module.
+
+Change Reservation#get_all_dates to #all_dates.
+
+Find a better way to organize test data.
+
+
+
+Notes from instructor feedback - 9.29.18:
+
+BookingManager and Calendar need the most work. The responsibilities need to be split differently, and they're too tightly coupled.
+
+BookingManager and Block are also tightly coupled.
+
+BookingManager needs a factory method to create Reservations and assign room numbers to them.
+
+One of the classes needs to store the reservations and their room numbers - I think it should be BookingManager.
+
+Replace StandardErrors with custom exceptions in BookingManager and Reservation.
+
+Refactor Reservation#number_of_nights.
+
+Change Block discount case statement to hash.
+
+
+
+Second submission notes - 10.1.18:
+
+Is there a better way to attach the room to the reservation?
+
+Is Calendar#reservations or BookingManager#reservations better for providing a list of reservations?
+
+Test data is cleaned up a bit, but could still use improvement.
+
+I think some methods and variables could be named better. 

spec/block_spec.rb

@@ -0,0 +1,50 @@
+require_relative 'spec_helper'
+
+describe Hotel::Block do
+  let(:calendar) {
+    Hotel::Calendar.new(20)
+  }
+  let(:block2) {
+    Hotel::Block.new('181202', '181206', 2)
+  }
+  let(:block3) {
+    Hotel::Block.new('181202', '181206', 3)
+  }
+  let(:block4) {
+    Hotel::Block.new('181202', '181206', 4)
+  }
+  let(:block5) {
+    Hotel::Block.new('181202', '181206', 5)
+  }
+  let(:block6) {
+    Hotel::Block.new('181202', '181206', 6)
+  }
+  let(:block1) {
+    Hotel::Block.new('181202', '181206', 1)
+  }
+  let(:reservation1) {
+    Hotel::Reservation.new('181202', '181204')
+  }
+
+  describe "#initalize" do
+    it "can be instantiated" do
+      expect(block2).must_be_kind_of Hotel::Block
+      rooms = block2.number_of_rooms
+    end
+    it "raises ArgumentError if number_of_rooms not in range" do
+      expect{Hotel::Block.new('181202', '181206', 6)}.must_raise ArgumentError
+      expect{Hotel::Block.new('181202', '181206', 1)}.must_raise ArgumentError
+    end
+    it "calculates cost with discount based on number_of_rooms" do
+      expect(block2.cost).must_equal 790
+      expect(block3.cost).must_equal 780
+      expect(block4.cost).must_equal 770
+      expect(block5.cost).must_equal 760
+    end
+    it "stores hash of rooms and status" do
+      expect(block2.rooms).must_be_kind_of Hash
+      expect(block2.rooms).must_be_empty
+    end
+  end
+
+end