Branches - Kelsey

.gitignore

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

design-activity.md

@@ -0,0 +1,58 @@
+What classes does each implementation include? Are the lists the same?
+Each implementation contains the same three classes.
+
+Write down a sentence to describe each class.
+class CartEntry: refers to an item with a unit price and a quantity.
+class ShoppingCart: refers to the shopping cart that contains many entries.
+class Order: refers to the total price of the items in the shopping cart.
+
+How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper.
+ShoppingCart has a one-to-many relationship with CartEntry. Order has a one-to-one relationship with ShoppingCart. Order has no direct relationship with CartEntry.
+
+What data does each class store? How (if at all) does this differ between the two implementations?
+CartEntry stores unit_price and quantity. ShoppingCart stores an array of entries (I assume instances of CartEntry). Order stores a cart (an instance of ShoppingCart).
+
+What methods does each class have? How (if at all) does this differ between the two implementations?
+In the first implementation, only the Order has any method. In the second implementation, both CartEntry and ShoppingCart have their own methods to calculate their price, and the Order class has a method to calculate the total_price.
+
+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?
+
+  In the first implementation, Order retains all the logic to compute the price. In the second implementation, each class is responsible for calculating its own price, and Order uses these calculations to decipher its total price.
+
+  Does total_price directly manipulate the instance variables of other classes?
+
+  In the first implementation, total_price directly accesses the instance variables. Not so in the second implementation.
+
+  If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify?
+
+  The second implementation is easier to modify because we could add a method inside the CartEntry class to calculate the bulk discount.
+
+  Which implementation better adheres to the single responsibility principle?
+  Implementation B.
+
+Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled?
+Implementation B.
+
+
+ACTIVITY
+
+(Note: I used "refactors.txt" incorrectly by writing the refactors I did while doing my project instead of potential refactors to come back to.)
+
+I believe my code follows the single responsibility principle fairly well. Other than in creating an instance of a class, none of my classes modify the instance variables of another class. My classes for Room, Calendar, Block, and Reservation are all loosely coupled. My Reservation_Manager class is quite stuffed and may have more than one responsibility: creating reservations *and* returning information about reservations and rooms.
+
+Two situations that I think could stand to use a separate class are 1) the method `calculate_total_cost` in both Block and Reservation could be its own class for computing the price, and 2) the helper methods in Reservation_Manager for retrieving information (i.e. `get_rooms_by_id`, `list_reservations_by_date`) could be in a separate class that's strictly for reading information and not manipulating information.
+
+I'm going to choose #2 from above and create two separate classes out of the Reservation_Manager: one to hold the helper methods that retrieve information about the rooms and reservations in the hotel, and one to hold the methods to create reservations. (I'll hopefully refactor the methods themselves along the way.)
+
+The current Reservation_Manager class will hold the informational methods, and the new class Reservation_Creator will hold the actionable methods.
+
+Reservation_Creator will be a child of Reservation_Manager, since it will utilize the same instance variables.
+
+I hope to copy/paste the majority of the code, and then refactor once the new class is functioning and passing all the tests.
+
+---
+
+About halfway into refactoring I deduced that this choice to separate the methods in "Reservation_Manager" was extremely unnecessary. It simply created back-and-forth coupling between the "Reservation_Manager" and the "Reservation_Creator". I do not think this a particularly efficient refactoring. I'm not sure what would be a better solution to the overstuffed ReservationManager class. While that class is technically responsible for more information processing and accessing than the rules of object-oriented design might allow for, I can't come up with a more efficient system of object organization. I would welcome suggestions on what I could have done better.
+
+As a result of this activity, I noticed that my tests were not optimally testing my code. I made some small refactors to a few of them to improve them.

lib/block.rb

@@ -0,0 +1,36 @@
+module Hotel
+
+  class Block < Reservation
+
+    attr_reader :start_date, :end_date
+    attr_accessor :id, :rooms, :dates, :discount, :reserved_rooms, :reservations
+
+    def initialize(start_date, end_date, room, discount: 0.8)
+      super(start_date, end_date, room)
+      @id = create_block_id
+      @rooms = room
+      @discount = discount
+      @reserved_rooms = []
+      @reservations = []
+    end
+
+    def create_block_id
+      "#{@start_date.year % 100}#{@start_date.month}#{@start_date.day}#{rand(9999)}B"
+    end
+
+    def add_reservation_to_block(reservation)
+      @reserved_rooms << reservation.room
+      @reservations << reservation
+      @rooms.delete reservation.room
+    end
+
+    def calculate_total_cost
+      total_cost = @reservations.map do |reservation|
+        @room = reservation.room
+        super 
+      end
+      total_cost.sum
+    end
+
+  end
+end

lib/calendar.rb

@@ -0,0 +1,40 @@
+require 'date'
+
+module Hotel
+
+  class Calendar
+
+    attr_reader :today
+
+    def initialize
+      @today = Date.today
+    end
+
+    def valid_start_date?(start_date)
+      # Does not accept reservations made on the current day
+      start_date > @today ? true : false
+    end
+
+    def valid_end_date?(start_date, end_date)
+      # Does not accept one-day reservations
+      start_date < end_date ? true : false
+    end
+
+    def overlap?(dates_to_check, reserved_dates)
+      reserved_dates.flatten!
+      (reserved_dates & dates_to_check).any? # if true, then there is overlap.
+    end
+
+    def get_date_range(start_date, end_date)
+      if !valid_start_date?(start_date)
+        raise ArgumentError, 'Start date cannot be current date.'
+      elsif !valid_end_date?(start_date, end_date)
+        raise ArgumentError, 'End date must be later than start date.'
+      else
+        # gets the date range between start & end dates, without the checkout date
+        (start_date...end_date).to_a
+      end
+    end
+
+  end  
+end

lib/reservation.rb

@@ -0,0 +1,35 @@
+require_relative 'calendar'
+require_relative 'reservation_manager'
+
+module Hotel
+
+  class Reservation
+
+    attr_reader :start_date, :end_date
+    attr_accessor :id, :room, :dates, :discount
+
+    def initialize(start_date, end_date, room, discount: 0)
+      @start_date = start_date
+      @end_date = end_date
+      @room = room
+      @id = create_id
+      @dates = get_all_dates
+      @discount = discount
+    end
+
+    def create_id
+      "#{@start_date.year % 100}#{@start_date.month}#{@start_date.day}#{rand(9999)}"
+    end
+
+    def get_all_dates
+      (@start_date...@end_date).to_a
+    end
+
+    def calculate_total_cost
+      rate = @discount == 0 ? @room.rate : @room.rate * @discount
+      rate * @dates.length
+    end
+
+  end
+
+end

lib/reservation_creator.rb

@@ -0,0 +1,81 @@
+require_relative 'reservation_manager'
+
+module Hotel
+
+  class ReservationCreator < Hotel::ReservationManager
+
+    attr_reader :reservation_manager
+
+    def initialize
+      super
+      @reservation_manager = Hotel::ReservationManager.new
+    end
+
+    def create_block(start_date, end_date, room)
+      block = Hotel::Block.new(start_date, end_date, room)
+      @reservation_manager.blocks << block
+      return block
+    end
+
+    def create_reservation(start_date, end_date, room, discount: discount)
+      reservation = Hotel::Reservation.new(start_date, end_date, room, discount: discount)
+      @reservation_manager.reservations << reservation
+      return reservation
+    end
+
+
+    def request_reservation(start_date, end_date, type: :room, block_id: nil, amount: 1)
+      if block_id
+      block_ids = @reservation_manager.blocks.map {|block| block.id}
+        if block_ids.include? block_id
+          if type == :block
+            raise ArgumentError, "You may not reserve a block within a block."
+          else
+            reserve_in_block(start_date, end_date, block_id)
+          end
+        else
+          raise ArgumentError, "That block does not exist."
+        end
+      else
+        available_rooms = @reservation_manager.find_available_rooms(start_date, end_date)
+        if type == :block 
+          if amount > 5 || amount < 1
+            raise ArgumentError, "You may only reserve up to 5 rooms."
+          elsif available_rooms.length < amount
+            raise ArgumentError, "There are not enough available rooms for those dates."
+          else 
+            create_block(start_date, end_date, available_rooms[0...amount])
+          end
+        elsif type == :room
+          if available_rooms.first == nil
+            raise ArgumentError, "There are no rooms available for those dates."
+          else
+            create_reservation(start_date, end_date, available_rooms.first)
+          end
+        end
+      end
+    end
+
+    def reserve_in_block(start_date, end_date, id)
+      block_to_reserve = @reservation_manager.blocks.find do |block| 
+        block.id == id
+      end
+      if start_date == block_to_reserve.start_date && end_date == block_to_reserve.end_date
+        room_to_reserve = @reservation_manager.find_available_rooms(start_date, end_date, rooms: block_to_reserve.rooms).first
+        if room_to_reserve == nil
+          raise ArgumentError, "There are no available rooms in this block."
+        else
+          reservation = create_reservation(start_date, end_date, room_to_reserve, discount: block_to_reserve.discount)
+          block_to_reserve.add_reservation_to_block(reservation)
+          return reservation
+        end
+      else
+        raise ArgumentError, "The reservation dates must be the same as the reserved block dates."
+      end
+    end
+
+
+  end
+
+
+end

lib/reservation_manager.rb

@@ -0,0 +1,53 @@
+require_relative 'room'
+
+module Hotel
+  class ReservationManager
+
+    attr_accessor :rooms, :reservations, :blocks
+
+    def initialize(rooms: Hotel::Room.load_all, reservations: [], blocks: [], calendar: Hotel::Calendar.new)
+      @rooms = rooms
+      @reservations = reservations
+      @blocks = blocks
+      @calendar = calendar
+    end
+
+
+
+    def find_available_rooms(start_date, end_date, rooms: @rooms)
+      search_range = @calendar.get_date_range(start_date, end_date)      
+      available_rooms_by_id = rooms.map { |room| room.id }
+
+      @reservations.each do |reservation|
+        if @calendar.overlap?(search_range, reservation.dates)
+          available_rooms_by_id = available_rooms_by_id - [reservation.room.id]
+        end
+      end
+      if available_rooms_by_id[0] == nil
+        return [nil]
+      else
+        available_rooms_by_id.map do |id|
+          get_rooms_by_id(id)
+        end
+      end
+    end
+
+    def get_rooms_by_id(id)
+      rooms_by_id = []
+      @rooms.find do |room|
+        if room.id == id
+          rooms_by_id << room
+        end
+      end
+    end
+
+    def list_reservations_by_date(date)
+      reservations = @reservations.map do |reservation|
+        # Currently includes end date in search parameters
+        reservation if (reservation.dates.include? date) || (reservation.end_date == date)
+      end
+      reservations.compact
+    end
+
+  end
+end

lib/room.rb

@@ -0,0 +1,26 @@
+module Hotel
+
+  class Room
+
+    TOTAL_ROOMS = 20
+    BASE_RATE = 200
+
+    attr_reader :id
+    attr_accessor :rate
+
+    def initialize(id)
+      @id = id
+      @rate = BASE_RATE
+    end
+
+    def self.load_all
+      rooms = []
+      TOTAL_ROOMS.times do |i| 
+        rooms << new(i+1)
+      end
+      return rooms
+    end
+
+  end
+
+end

refactor.txt

@@ -0,0 +1,15 @@
+NOTES
+
+REFACTORS: 
+
+Removed Date.parse dates scattered throughout code. Made Date objects for tests. My code assumes that all dates entered into the program will be instances of Date.
+
+Reservation 16: originally had a variable to hold Room Rate, removed in favor of retrieving the rate from the instance of Room
+
+Went back and added ids to Reservations and Blocks.
+
+Res_Man 43: added if/else statement to be less expensive and not call "get_rooms_by_id" if there are no rooms.
+
+Moved "discount" from Block to Reservations class.
+
+Removed "list_discounted_rates" from Block, revamped "calculate_total_cost" instead.