Branches - Angele

.gitignore

@@ -1,3 +1,5 @@
+coverage
+
 *.gem
 *.rbc
 /.config

design-activity.md

@@ -0,0 +1,15 @@
+| Prompt | Answer |
+|--- |--- |
+|What classes does each implementation include? Are the lists the same?| They both have the same classes|
+|Write down a sentence to describe each class.|CartEntry is the template for a single cart entry item's unit price and number added to the cart.  ShoppingCart contains all the info you can get from looking at a shopping cart, ie. how many CartEntries there are, and from there, how many of each and their price. Order contains how much the total order costs and initializes an empty 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.|In implementation A, the total cost of the order is calculated with what is basically a train wreck by telling the cart and cart entries how to return back the subtotal. In implementation B, the calculation is basically spread out among the classes so that each returns the piece of information it should be responsible for calculating or giving. Finally, total_price only asks the cart to tell its subtotal, and the Order class knows how much sales tax is, so it only needs to add the sales tax to the subtotal.|
+|What data does each class store? How (if at all) does this differ between the two implementations?| drawing a diagram of both implementations helped me see that each essentially stores the same data, but provides it in better formats in implementation B. |
+|What methods does each class have? How (if at all) does this differ between the two implementations?| The methods are different between the two implementations, this is how the data is provided and accessed by other classes - the methods basically access their own data to provide data to other methods in a ready-to-use format.|
+|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? Does total_price directly manipulate the instance variables of other classes?| In implementation A, logic to compute the price is determined by the total_price method directly accessing the instance variables of other classes. In implementation B, it accesses instead the return value of only Cart's price method to get the subtotal, and Cart gets its entry prices by asking each CartEntry for the info, without going directly into each CartEntry's instance variables.|
+|If we decide items are cheaper if bought in bulk, how would this change the code? Which implementation is easier to modify?| If we decide items are cheaper bought in bulk and were working with implementation A, we would have to put all of the logic into the Order total_price method, which would check the quantity of each CartEntry and do changes if necessary. It is easier to modify implementation B's CartEntry class since it can be done in the price method.|
+|Which implementation better adheres to the single responsibility principle?|Implementation B definitely adheres better to the single responsibility principle since there is never any overreaching method that accesses another's instance variables.|
+|Bonus question once you've read Metz ch. 3: Which implementation is more loosely coupled?| Definitely implementation B since each class doesn't really know how the other one functions internally, they are able to work together by asking each other for information and not telling eachother how to work or relying on another's instance variables from outside of the class.|
+
+## Hotel Refactor
+#### Where does a class take on multiple roles, or directly modify the attributes of another class?
+Room's function is to be a room with a rate, number, and list of reservations and blocks it has. It currently also has to say if it is available for a given date range by looking at its own reservation and block check-in, check-out dates. Room.status() tells both reservations AND blocks _how_ to determine if thre is a check-in/check-out conflict by accessing their instance variables. It should instead just ask each block and reservation if they are available for given date ranges. To change it, I would have to basically move the logic from Room into both Reservation and Block, but since it's basically the same logic for each, I would rather only write it in one location, so I'll make a new DateRange class that deals with the logic and just returns true or false if there's either a check-in conflict or a check-out conflict, and Room will still say whether its status is :AVAILABLE or :UNAVAILABLE. This will be an improvement on single responsibility and the same thing won't need to be written twice anywhere.

lib/block.rb

@@ -0,0 +1,24 @@
+require_relative 'date_range'
+
+module Hotel
+  class Block < Hotel::DateRange
+    attr_reader :id, :check_in_date, :check_out_date, :rooms, :available_rooms
+    def initialize(blocks_length:, check_in_date:, check_out_date:, rooms:, discount_rate:)
+      super(check_in_date, check_out_date)
+      @id = blocks_length + 1
+      @rooms = rooms
+      @discount_rate = discount_rate
+      @available_rooms = rooms
+    end
+
+    def choose_room
+      @available_rooms.shuffle!
+      return @available_rooms.pop
+    end
+
+    def date_range_conflict?(desired_check_in, desired_check_out)
+      super(desired_check_in, desired_check_out)
+    end
+
+  end
+end

lib/date_range.rb

@@ -0,0 +1,22 @@
+module Hotel
+  class DateRange
+    class DateRangeError < StandardError; end
+
+    def initialize(check_in_date, check_out_date)
+      @check_in_date = check_in_date
+      @check_out_date = check_out_date
+    end
+
+    def date_range_conflict?(desired_check_in, desired_check_out)
+      if desired_check_in.between?(@check_in_date, @check_out_date - 1)
+        return true
+      elsif
+        desired_check_out.between?(@check_in_date + 1, @check_out_date)
+        return true
+      else
+        return false
+      end
+    end
+
+  end
+end

lib/management_system.rb

@@ -0,0 +1,112 @@
+module Hotel
+  class ManagementSystem
+    class SystemReservationError < StandardError; end
+    attr_reader :rooms, :reservations, :blocks
+
+    def initialize
+      @NUMBER_OF_ROOMS = 20
+      @rooms = initialize_rooms
+      @reservations = []
+      @blocks = []
+    end
+
+    def initialize_rooms
+      initial_rooms = []
+      @NUMBER_OF_ROOMS.times do
+        initial_rooms << Hotel::Room.new(initial_rooms.length)
+      end
+      return initial_rooms
+    end
+
+    def list_rooms
+      return @rooms
+    end
+
+    def reservations_by_date(date)
+      return @reservations.select do |reservation|
+        date.between?(reservation.check_in_date,reservation.check_out_date)
+      end
+    end
+
+    def create_block(check_in_date:, check_out_date:, room_numbers:, discount_rate:)
+      if room_numbers.length < 1 || room_numbers.length > 5
+        raise SystemReservationError.new("Cannot create block for less than 1 or more than 5 rooms")
+      end
+      rooms_from_numbers = room_numbers.map do |room_number|
+        @rooms.find do |room|
+          room.number == room_number
+        end
+      end
+
+      room_statuses = rooms_from_numbers.map do |room|
+        room.status(desired_check_in: check_in_date, desired_check_out: check_out_date)
+      end
+
+      if room_statuses.any?(:UNAVAILABLE)
+        raise SystemReservationError.new("Cannot create block for rooms that are already reserved or blocked")
+      end
+
+      new_block = Hotel::Block.new(
+        blocks_length: @blocks.length,
+        check_in_date: check_in_date,
+        check_out_date: check_out_date,
+        rooms: rooms_from_numbers,
+        discount_rate: discount_rate
+      )
+      @blocks << new_block
+      rooms_from_numbers.each do |room|
+        room.blocks << new_block
+      end
+      return new_block
+    end
+
+    def reserve_blocked_room(block_id)
+      block = find_block_by_id(block_id)
+      chosen_room = block.choose_room
+      if chosen_room == nil
+        raise SystemReservationError.new("There are no available rooms for the chosen block")
+      end      
+      create_reservation(
+        check_in_date: block.check_in_date, 
+        check_out_date: block.check_out_date, 
+        room: chosen_room
+      )
+    end
+
+    def find_block_by_id(block_id)
+      @blocks.find { |block| block.id == block_id }
+    end
+
+    def create_reservation(check_in_date:, check_out_date:, room: choose_room(check_in_date, check_out_date))
+      new_reservation = Hotel::Reservation.new(
+        check_in_date: check_in_date, 
+        check_out_date: check_out_date,
+        room: room
+      )
+      @reservations << new_reservation
+      new_reservation.room.reservations << new_reservation
+    end
+
+    def choose_room(desired_check_in, desired_check_out)
+      available_rooms = available_rooms_for(desired_check_in, desired_check_out)
+      return available_rooms.sample
+    end  
+
+    def available_rooms_for(desired_check_in, desired_check_out)
+      available_rooms = @rooms.select do |room|
+        room.status(desired_check_in: desired_check_in, desired_check_out: desired_check_out) == :AVAILABLE        
+      end
+      if available_rooms.length > 0
+        return available_rooms
+      else
+        raise SystemReservationError.new("There are no available rooms for the desired_check_in and desired_check_out dates")
+      end
+    end
+
+    def available_rooms_by_block(block_id)
+      block = find_block_by_id(block_id)
+      return block.available_rooms
+    end
+
+  end
+end

lib/reservation.rb

@@ -0,0 +1,33 @@
+require_relative 'date_range'
+
+module Hotel
+  class Reservation < Hotel::DateRange
+    class ReservationDateError < StandardError; end
+    attr_reader :check_in_date, :check_out_date, :room
+    def initialize(check_in_date:, check_out_date:, room:)
+      super(check_in_date, check_out_date)
+      @room = room
+
+      if @check_in_date < Date.today 
+        raise ReservationDateError.new("check_in_date cannot be before today's date")
+      elsif @check_out_date < @check_in_date
+        raise ReservationDateError.new("check_out_date cannot be before check_in_date")
+      end
+
+    end
+
+    def total_cost
+      nightly_rate = @room.nightly_rate
+      return nightly_rate * total_nights
+    end
+
+    def total_nights
+      return @check_out_date - @check_in_date
+    end
+
+    def date_range_conflict?(desired_check_in, desired_check_out)
+      super(desired_check_in, desired_check_out)
+    end
+
+  end
+end

lib/room.rb

@@ -0,0 +1,28 @@
+module Hotel
+  class Room
+    attr_reader :reservations, :nightly_rate, :number, :blocks
+    attr_reader :lists_to_check_status_on
+
+    def initialize(rooms_length)
+      @reservations = []
+      @nightly_rate = 200
+      @number = rooms_length + 1
+      @blocks = []
+      @lists_to_check_status_on = [@reservations, @blocks]
+    end
+
+    def status(desired_check_in:, desired_check_out:)
+      status = :AVAILABLE
+      @lists_to_check_status_on.each do |list|
+        list.each do |reservation_or_block|
+          if reservation_or_block.date_range_conflict?(desired_check_in, desired_check_out)
+            status = :UNAVAILABLE 
+            break
+          end
+        end
+      end
+      return status
+    end
+
+  end
+end

refactors.txt

@@ -0,0 +1,56 @@
+***********   ***********   ***********   ***********   ***********   
+- I should pull out the date range checking parts into
+a separate class, DateRange that checks for validity and
+does comparisions (i think?) .. (i think that's what a 
+lot of other people did?)
+  -9/29/19 revisit notes: I think separating this out would definitely help increase single responsibility in the program.
+***********   ***********   ***********   ***********   ***********   
+
+- I should reconsider whether there are good places to
+use class methods or variables instead of instance 
+methods or variables
+  -9/29/19 revisit notes: Not sure, will try to think about this while refactoring
+
+- I should determine whether it's better to keep the 
+room nightly_rate stored as a variable in each room 
+or in the ManagementSystem
+  -9/29/19 revisit notes: I could set it stored in management system, and then each room has a default "rate_multiplier" variable that is defaulted to "1", then if the price is different... idk. I don't think it will make it easier to change room prices this way. I think I will leave it as is for now since there is no need to change room prices except for blocked rooms which just take in a discount rate.
+
+- For setting rates of rooms on instantiation I should
+think about maybe a CLI that asks for info or a CSV 
+that stores number of rooms, room numbers, nightly rate
+  -9/29/19 revisit notes: no need to think about CLI right now.
+
+***********   ***********   ***********   ***********   ***********   
+- I need to change the status method in Room to a separate 
+method, maybe moved into the future DateRange class, or maybe 
+still in Room but separated out, so that I don't have to use 
+basically a copy pasted version of checking conflicts between 
+reservations and checking conflicts between dates.
+  -9/29/19 revisit notes: Yes I definitely want to fix this, I think it should go into the future DateRange class.
+***********   ***********   ***********   ***********   ***********   
+
+- I think there's a better way to increase room/reservation/
+block numbers instead of requiring the current length of rooms/
+reservations/blocks being passed in, maybe those @@ variables 
+Eve was showing me, or maybe something completely different, but 
+for some reason I don't really like the way it looks with for
+example Room instantiation needing the current length of rooms
+to be passed in. IDK maybe it's fine.
+  -9/29/19 revisit notes: I think it's fine for now.
+
+
+- I feel like create_block is a really big method and I maybe 
+created it too hastily, and it looks kind of messy and I bet it
+can be trimmed down or moved into the Block class maybe? or at 
+least trimmed down. I also think it's pretty readable right now 
+though so I want to make sure not to lose that part of it.
+  -9/29/19 revisit notes: yeah, I am not sure if it should be moved into the block class or not .... hmm. I think it belongs where it is but I'm not sure.
+
+***********   ***********   ***********   ***********   ***********   
+- A lot of times I wasn't sure whether I should make methods in
+Room/Block/Reservation or in ManagementSystem, so I would like to 
+re-look at that after seeing other people's Hotels and learning 
+more about OOP.
+  -9/29/19 revisit notes: from the Revisit Hotel readme it sounds like it's a noob thing to do to put so many methods in ManagementSystem. I'll try to look at it and see how things can be moved around. I should try to make it so that ManagementSystem asks the other classes what to do, and doesn't tell them how to do anything.
+***********   ***********   ***********   ***********   ***********   

test/block_test.rb

@@ -0,0 +1,20 @@
+require_relative 'test_helper'
+
+describe "block class" do
+  before do
+    @room_1 = Hotel::Room.new(0)
+    @room_2 = Hotel::Room.new(1)
+
+    @block_1 = Hotel::Block.new(
+      blocks_length: 0,
+      check_in_date: Date.today,
+      check_out_date: Date.today + 1,
+      rooms: [@room_1, @room_2],
+      discount_rate: 150
+    )
+  end
+
+  it "can initialize" do
+    expect(@block_1).must_be_instance_of Hotel::Block
+  end
+end