Knapsack class in Ruby This is a simple implementation of a knapsack written in Ruby.  I've included all the relevant code here, but my question is concerning the contents setter .contents=

## item.rb
class Item
  attr_accessor :weight
  attr_accessor :data

  def initialize(weight:, data: nil)
    @weight = weight
    @data   = data
  end

end




 ## knapsack.rb
class KnapsackCapacityExceededError &lt; StandardError; end
class KnapsackWeightExceededError &lt; StandardError; end
class KnapsackContentError &lt; StandardError; end

require_relative('./item')

class Knapsack
  attr_reader :capacity
  attr_reader :weight
  attr_reader :contents

  def initialize(capacity:, weight:)
    @capacity = capacity
    @weight   = weight
    @contents = Array.new(@capacity) { nil }
  end

  def contents=(new_contents)
    raise KnapsackCapacityExceededError if self.exceeds_capacity? new_contents
    raise KnapsackWeightExceededError   if self.exceeds_weight? new_contents
    raise KnapsackContentError          unless new_contents.all? { |e| e.is_a? Item }

    @contents = new_contents
  end

  def fit?(contents)
    return false if self.exceeds_weight?(contents) || self.exceeds_capacity?(contents)
    true
  end
  alias_method :fits?, :fit?

  def fits_weight?(contents)
    new_weight = contents.map { |item| item.weight }.sum
    return true if new_weight &lt;= self.weight
    false
  end

  def exceeds_weight?(contents)
    return true if !fits_weight? contents
    false
  end

  def fits_capacity?(contents)
    return true if contents.length &lt;= self.capacity
    false
  end

  def exceeds_capacity?(contents)
    return true if !fits_capacity? contents
    false
  end

end

# k = Knapsack.new(capacity: 10, weight: 50)


In this method there are three conditions where an exception is raised, is this a code smell?

Any other feedback also appreciated.
So there are multiple things. First of all I'll share the code refactoring related suggestions and then we can look at the performance improvements of code.

Refactoring


You don't really need to pass a block to Array.new to initialize the array with nil values.

def initialize(capacity:, weight:)
  @capacity = capacity
  @weight   = weight
  @contents = Array.new(@capacity) { nil }
end


can be refactored to:

def initialize(capacity:, weight:)
  @capacity = capacity
  @weight   = weight
  @contents = Array.new(@capacity)
end

def contents=(new_contents)
  raise KnapsackCapacityExceededError if self.exceeds_capacity? new_contents
  raise KnapsackWeightExceededError   if self.exceeds_weight? new_contents
  raise KnapsackContentError          unless new_contents.all? { |e| e.is_a? Item }

  @contents = new_contents
end


can be refactored to:

def contents=(new_contents)
  raise KnapsackCapacityExceededError if exceeds_capacity? new_contents
  raise KnapsackWeightExceededError   if exceeds_weight? new_contents
  raise KnapsackContentError          if has_non_items? new_contents

  @contents = new_contents
end

def has_non_items?(contents)
  contents.any? { |content| !content.is_a?(Item) }
end


-> Notice that usage of self keyword has been removed.

-> Moved the logic of checking any non-items in contents to a separate method. That makes the if conditions more readable and it replaces the unless with if to make it consistent with rest of the conditions.
Method fit? can be refactored to:

def fit?(contents)
  fits_weight?(contents) &amp;&amp; fits_capacity?(contents)
end

Method fits_weight? can be refactored as well.

def fits_weight?(contents)
  contents.map(&amp;:weight).sum &lt;= weight
end



Performance Tuning

You don't need to initialize the @contents array with nil elements. You can just write @contents = [] to save some extra used memory.