hotel.rb

Guardfile

@@ -1,4 +1,4 @@
-guard :minitest, bundler: false, rubygems: false do
+guard :minitest, bundler: false, autorun: true, 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,75 @@
+1. What classes does each implementation include? Are the lists the same?
+Each implementation includes the classes: CartEntry, ShoppingCart, and Order.
+No, the lists aren't the same. In A, the instance variable @entries is both readable and writable. In B, @entries is not readable or writable.
+
+
+
+2. Write down a sentence to describe each class.
+CartEntry --- it is the blueprint for new CartEntry objects, each object will be instantiated with @unit_price and @quantity attributes. (In A, the attributes are both readable and writable, but not in B.)
+
+ShoppingCart --- it contains the list of entries as an instance variable @entries.
+
+Order --- it is the blueprint for Order objects, each order is instantiated with a @cart attribute which is a list of @entries. (Each time an Order object is instantiated, it calls ShoppingCart.new and assigns it to the local variable @cart.)  
+
+
+3. How do the classes relate to each other? It might be helpful to draw a diagram on a whiteboard or piece of paper.
+?? how can @cart call on entries? i thought it was set to equal ??
+
+CartEntry is the factory for all entries, ShoppingCart stores all entries as a list, Order has a method that iterates through the list of entries, and performs a math operation on each entry to find the total price of an Order (by multiplying entry.unit_price by entry.quantity then summing the price of all entries).
+
+
+
+4. What data does each class store? How (if at all) does this differ between the two implementations?
+ShoppingCart is the only class that stores data, it stores a list of entries.
+In A, the instance variable @entries stores the list of entries and it is both readable and writable outside of the ShoppingCart class.
+In B, the instance variable @entries stores the list of entries but is not accessible outside of the ShoppingCart class.
+
+
+
+5. What methods does each class have? How (if at all) does this differ between the two implementations?
+Both classes have initialize methods which initialize CartEntry with @unit_price and @quantity, ShoppingCart with @entries as an array, and Order with @cart as ShoppingCart.new(which is basically an array).
+
+In B, CartEntry has an additional method that A doesn't have called #price which performs a math operation between @unit_price and @quantities (instance variables defined in its own class) and returns the price of an entry.
+In B, ShoppingCart also has a method that A doesn't have called #price which iterates through the list of @entries (the instance variable defined in its own class) to add up and return the sum of all entries.
+The Order class in A and B both contain #total_price. In A, the instance variable @cart needs call on entries to then iterate through each entry in order to first view their unit_price and quantities and then multiply by the tax to perform the math operation to find the total price of the order. In B, the instance variable @cart needs to just call price then multiply by the tax to find the total price.
+
+
+6. Consider the Order#total_price method. In each implementation:
+6.1) Is logic to compute the price delegated to "lower level" classes like ShoppingCart and CartEntry, or is it retained in Order? In A it is retained in Order, in B it is delegated.
+
+6.2) Does total_price directly manipulate the instance variables of other classes? Yes it does in A but not in B.
+
+6.3) If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify? B is easier to modify.
+
+6.4) Which implementation better adheres to the single responsibility principle?
+Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled?
+B is more loosely coupled and adheres better to the single responsibility principle.
+
+<!--~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~-->
+~~ Refactor Activity: ~~
+Based on the answers to each set of the above questions, identify one place in your Hotel project where a class takes on multiple roles, or directly modifies the attributes of another class. Describe in design-activity.md what changes you would need to make to improve this design, and how the resulting design would be an improvement.
+
+One design issue I have in TrackingSystem is that it has too many jobs. One design change I can make to improve this is by moving some date checking logic out of this class and into the Reservation class. In TrackingSystem I'm currently iterating through the list of reservations in order to be able to perform
+date checking logic, and it could just be done in the Reservation class itself. Moving the date checking logic to the Reservation class will be an improvement because the TrackingSystem will need to know less about the structure and variables of the Reservation class and it will DRY up the code in certain methods that iterate through
+the list of reservations.
+
+Extra notes...
+
+Questions to keep in mind:
+-should the instance variables be readable and writable?
+-should there be a class thats job is just to store data?
+-can some computing logic be delegated to lower classes?
+
+1. The TrackingSystem class takes on multiple roles
+  a. it creates all the rooms
+  b. it stores the list of blocks, list of reservations, and list of rooms
+  *c*. it contains date checking logic that checks on a reservation to determines a room and block's availability
+  d. it adds new blocks and new reservations to its data
+
+2. The TrackingSystem directly modifies the attributes of
+  a. a room's reserved dates and its block status
+  b. a reservation's price
+  c. a block's reserved dates, block status and rooms inside it
+
+3. The Room class takes on multiple roles
+  a. it keeps track of a list of dates that it is unavailable which is doing too much it should let the TrackingSystem class be the only place where reservations data is kept. I realized that most of the methods in TrackingSystem are going through the Room objects to check the reservation status, when it should be checking that through Reservation objects. 

lib/block.rb

@@ -0,0 +1,13 @@
+class Block
+  attr_reader :start_time, :end_time, :rooms
+  attr_accessor :block, :discount
+
+  def initialize(attributes)
+    @rooms = attributes[:rooms]
+    @start_time = attributes[:start_time]
+    @end_time = attributes[:end_time]
+    @discount = attributes[:discount]
+    @block = attributes[:block] ||= :NA
+  end
+
+end

lib/reservation.rb

@@ -0,0 +1,51 @@
+class Reservation
+  attr_reader :room_num, :start_time, :end_time, :price
+
+  def initialize(attributes)
+    @room_num = attributes[:room_num]
+    @start_time = attributes[:start_time]
+    @end_time = attributes[:end_time]
+    @price = attributes[:price]
+  end
+
+  def total_cost
+    total_cost = 0
+    return total_cost = ((end_time - start_time) * price).round(2)
+  end
+
+  def date_range
+    return self.start_time...self.end_time
+  end
+  def reservations_on(date)
+    raise ArgumentError.new"#{date} must be instance of Date" unless date.instance_of? Date
+    reservations_on_date = []
+    TrackingSystem.reservations.each do |reservation|
+      if reservation.include?(date)
+        reservations_on_date << reservation
+      end
+    end
+    if reservations_on_date.empty?
+      raise ArgumentError.new"There are no reservations on #{date}"
+    else
+      return reservations_on_date
+    end
+  end
+
+  def ranges_overlap?(other_date_range)
+    self.date_range.include?(other_date_range.first) || other_date_range.include?(self.start_time)
+  end
+
+  def include?(date)
+    if self.date_range.include?(date)
+      return true
+    else
+      return false
+    end
+  end
+
+  def number_of_nights
+    return (self.end_time - self.start_time).to_i
+  end
+
+
+end

lib/room.rb

@@ -0,0 +1,10 @@
+class Room
+  attr_reader :room_num, :reserved_dates
+  attr_accessor :block
+
+  def initialize(attributes)
+    @room_num = attributes[:room_num]
+    @reserved_dates = attributes[:reserved_dates]
+    @block = attributes[:status] ||= :NA 
+  end
+end

lib/tracking_system.rb

@@ -0,0 +1,163 @@
+require_relative 'room'
+require_relative 'reservation'
+require_relative 'block'
+require 'pry'
+
+NUMBER_OF_ROOMS = 20
+STANDARD_ROOM_PRICE = 200.00
+
+class TrackingSystem
+  attr_reader :all_rooms, :reservations, :blocks
+
+  def initialize
+    @all_rooms = add_rooms
+    @reservations = [] #when a block is created, would i instead add that number of rooms into the reservations list & change their block status?, and then when it's reserved add one set of {dates} to the reservation
+    @blocks = []
+  end
+
+
+  def add_rooms
+    all_rooms = []
+    NUMBER_OF_ROOMS.times do |i|
+      i += 1
+      i = Room.new({room_num: i, reserved_dates: []})
+      all_rooms << i
+    end
+    return all_rooms
+  end
+
+
+  def view_all_rooms
+    return @all_rooms
+  end
+
+
+
+  def total_cost_of_reservation(reservation)
+    raise ArgumentError.new"#{reservation} must be instance of Reservation" unless reservation.instance_of? Reservation
+    return reservation.total_cost
+  end
+
+  def view_available_rooms_on(start_time: Date.now, end_time: Date.now + 1)
+    raise ArgumentError.new"start_time must be before end_time" unless start_time < end_time
+    available_rooms = []
+    unavailable_count = 0
+    @all_rooms.each do |room|
+      if room.reserved_dates.empty? && room.block == :NA
+        available_rooms << room
+      else
+        room.reserved_dates.each do |dates_hash|
+          if ranges_overlap?((dates_hash[:start_time]...dates_hash[:end_time]).to_a, (start_time..end_time).to_a) == false && room.block == :NA
+            available_rooms << room
+          else
+            unavailable_count += 1
+          end
+        end
+      end
+    end
+    if unavailable_count == NUMBER_OF_ROOMS
+      raise ArgumentError.new"no rooms available on that date range"
+    else
+      return available_rooms
+    end
+  end
+
+
+  def add_reservation(start_time: Date.now, end_time: Date.now + 1, number_of_rooms: 1)
+    available_rooms = view_available_rooms_on(start_time: start_time, end_time: end_time)
+    raise ArgumentError.new"Not enough rooms available on those dates" if available_rooms.length < number_of_rooms
+    number_of_rooms.times do |i|
+      @reservations << Reservation.new({room_num: available_rooms[i].room_num, start_time: start_time, end_time: end_time, price: 200.00})
+      available_rooms[i].reserved_dates << {start_time: start_time, end_time: end_time}
+    end
+    @reservations
+  end
+
+
+  def add_block(start_time: Date.today + 7, end_time: Date.today.next_month, number_of_rooms: 5, discount: 10)
+    raise ArgumentError.new"discount rate must be integer" unless discount.instance_of? Integer
+    raise ArgumentError.new"start_time must be before end_time" unless start_time < end_time
+    raise ArgumentError.new"number_of_rooms must be >= 1 && <= 5" unless number_of_rooms >= 1 && number_of_rooms <=5
+    available_rooms = view_available_rooms_on(start_time: start_time, end_time: end_time)
+    raise ArgumentError.new"not enough available rooms for this date range" if available_rooms.length < number_of_rooms
+    block_id = generate_block_id.to_sym
+    block = Block.new({rooms: [], start_time: start_time, end_time: end_time, discount: discount, block: block_id })
+    number_of_rooms.times do |i|
+      available_rooms[i].block = block_id
+      block.rooms << available_rooms[i]
+    end
+    @blocks << block
+    return @blocks
+  end
+
+  def retrieve_block_dates(block_id)
+    raise ArgumentError.new"#{block_id} must be a Symbol" unless block_id.instance_of? Symbol
+    dates_hash = {}
+    range_array = []
+    @blocks.each do |block|
+      if block.block == block_id
+        dates_hash[:start_time] = block.start_time
+        dates_hash[:end_time] = block.end_time
+        range_array = (dates_hash[:start_time]...dates_hash[:end_time]).to_a
+      end
+    end
+    raise ArgumentError if dates_hash.empty?
+    return range_array
+  end
+
+  def rooms_available_in_block(block_id)
+    raise ArgumentError.new"#{block_id} must be a Symbol" unless block_id.instance_of? Symbol
+    available_rooms = []
+    @blocks.each do |individual_block|
+      if individual_block.block == block_id
+        individual_block.rooms.each do |room|
+          if room.reserved_dates.empty?
+            available_rooms << room
+          else room.reserved_dates.each do |dates_hash|
+            if (dates_hash[:start_time]...dates_hash[:end_time]).to_a.sort != retrieve_block_dates(block_id).sort
+              available_rooms << room
+            end
+          end
+        end
+      end
+    end
+  end
+  return available_rooms
+end
+
+
+ def add_reservation_in_block(start_time: Date.now, end_time: Date.now + 1, number_of_rooms: 1, block: :NA)
+  available_rooms = rooms_available_in_block(block)
+  raise ArgumentError.new"Not enough rooms available on those dates" if available_rooms.length < number_of_rooms
+  discount = retrieve_block_discount(block)
+  number_of_rooms.times do |i|
+    @reservations << Reservation.new({room_num: available_rooms[i].room_num, start_time: start_time, end_time: end_time, price: STANDARD_ROOM_PRICE - (STANDARD_ROOM_PRICE * discount)})
+    available_rooms[i].reserved_dates << {start_time: start_time, end_time: end_time}
+  end
+  @reservations
+ end
+
+ def retrieve_block_discount(block_id)
+  discount = 0
+  @blocks.length.times do |i|
+    if @blocks[i].block == block_id
+      discount = (@blocks[i].discount / 100)
+    end #
+  end
+  return discount
+ end
+
+
+private
+
+ def generate_block_id
+  (0..3).map { (65 + rand(26)).chr }.join
+ end
+
+ def ranges_overlap?(r1, r2)
+  r1.include?(r2.first) || r2.include?(r1.first)
+ end
+
+
+
+end

refactors.txt

@@ -0,0 +1,12 @@
+1. name change --> .block method to .block_id in the Block, and Room classes
+2. the program logic surrounding blocks may be wrong, not sure if I understood the requirements correctly for: "If a room is set aside in a block, it is not available for reservation by the general public, nor can it be included in another block"
+   Does this mean if rooms are in a block that they cannot be reserved for that period of time?
+3. possibly create a helper method that returns two dates as a range --> def view_two_dates_as_range()  #<--put params {checkin_time: checkin, checkout_time: checkout}
+4. create a test data file with test data so that I have an visualization of how the information is coming in/should be modified for the program
+5. clean up the unnecessary test variables and combine them into a single "before do" block
+6. write more edge cases for each method in TrackingSystem
+7. for the methods that begin with title "view....", change to "return.." because they're actually returning arrays/values
+8. refactor methods in TrackingSystem that know about the structure of Block, Reservation, Room 's instance variables, a lot of them are going into arrays and hashes
+9. create helper methods for iterating through block lists and reservation lists and dates
+10. create helper methods to lessen the number of loops in methods
+11. use enumerables rather than iterating with .each to avoid lengthy loops 

spec/block_spec.rb

@@ -0,0 +1,28 @@
+require_relative 'spec_helper'
+require_relative '../lib/block'
+
+describe 'Block class' do
+
+  describe "#initialize" do
+    before do
+      attributes = {rooms: [] ,start_time: Date.new(2018,8,1),end_time: Date.new(2018,9,1), discount: 10, block: :A}
+      @block = Block.new(attributes)
+    end
+
+    it 'is an instance of Block' do
+      expect(@block).must_be_kind_of Block
+    end
+
+    it "is set up for specific attributes and data types" do
+      [:rooms, :start_time, :end_time, :discount, :block].each do |attribute|
+        expect(@block).must_respond_to attribute
+      end
+
+      expect(@block.rooms).must_be_kind_of Array
+      expect(@block.start_time).must_be_kind_of Date
+      expect(@block.end_time).must_be_kind_of Date
+      expect(@block.discount).must_be_kind_of Integer
+      expect(@block.block).must_be_kind_of Symbol
+    end
+  end
+end