Is there a a function in Ruby to increment an objects variable inside an array in this example?

Viewed 182

The drop1.amount or drop2.amount of Drop object in this example will not increase after the first time theyre run through.

class Drop
  attr_accessor :item, :price, :amount
end


drop1 = Drop.new()
drop1.item = "item1"
drop1.price = 2247
drop1.amount = 1

drop2 = Drop.new()
drop2.item = "item2"
drop2.price = 4401
drop2.amount = 60

x = 0
array = []
while x < 10
  rand1 = rand(2)

  if rand1 == 0
    if array.include? drop1.item
      drop1.amount = drop1.amount + 1
    else
      array << drop1.item
      array << drop1.amount
    end

  elsif rand1 == 1
    if array.include? drop2.item
      drop2.amount = drop2.amount + 60
    else
      array << drop2.item
      array << drop2.amount
    end
  end

  x += 1
end

puts array.to_s.gsub('"', '').gsub('[', '').gsub(']', '')
puts ""
puts drop1.amount
puts drop2.amount

Example of expected output:

item2, 240, item1, 6

6
240

Example of actual result:

item2, 60, item1, 1

6
240

I am looking for a change to the else statements in lines 24 and 32. The purpose of the this program is to create an array of items that will display the "item" one time and an incremented "amount" when a drop is randomly chosen multiple times.

2 Answers

array << drop1.amount does not make an alias of drop1.amount, it makes a one-time copy of the number value contained in drop1.amount. When you update drop1.amount, the copy in array is unchanged. Instead, put a reference to the object onto the result array or update the result array value directly (depending on whether you want to modify the original or not).

For example we can stick to the existing design with something like:

# ...
if array.include? drop1.item
  array[array.index(drop1.item)+1] += 1
  drop1.amount += 1 # optionally update the original (less ideal than an alias)
else
  array << drop1.item
  array << drop1.amount
end
# ...
if array.include? drop2.item
  array[array.index(drop2.item)+1] += 60
  drop2.amount += 60   
else
  array << drop2.item
  array << drop2.amount
end
# ...

While this emits the expected output, this sort of awkward searching and repeated code suggests that there are fundamental design flaws.

I'd write the program something like:

Drop = Struct.new(:item, :price, :amount)

drops = [
  Drop.new("item1", 2247, 1),
  Drop.new("item2", 4401, 60)
]
increment_amounts = drops.map {|e| e.amount}
result = [nil] * drops.size

10.times do 
  choice = rand(drops.size)

  if result[choice]
    result[choice].amount += increment_amounts[choice]
  else
    result[choice] = drops[choice]
  end
end

puts result.compact
           .shuffle
           .flat_map {|e| [e.item, e.amount]}
           .to_s
           .gsub(/["\[\]]/, "")
puts "\n" + drops.map {|e| e.amount}.join("\n")

Suggestions which the above version illustrates:

  • Use a struct instead of a class for such a simple type and set its properties using the constructor rather than accessors.
  • Use arrays instead of thing1, thing2, etc. This makes it a lot easier to make random choices (among other things). Note that the above version is expandable if you later decide to add more drops. After adding a third or 100 drops (along with corresponding increment amounts), everything just works.
  • Prefer a clear name like result instead of a generic name like array.
  • x = 0 ... while x < 10 ... x += 1 is clearer as 10.times.
  • Pass a regex to gsub instead of a string to avoid chaining multiple calls.

I don't know if I undestand properly the logic, but consider using a Hash instead of an Array:

h = {}
10.times do |x|
  rand1 = rand(2)

  if rand1 == 0
    if h.has_key? drop1.item
      drop1.amount += 1
      h[drop1.item] = drop1.amount
    else
      h[drop1.item] = drop1.amount
    end

  elsif rand1 == 1
    if h.has_key? drop2.item
      drop2.amount += 60
      h[drop2.item] = drop2.amount
    else
      h[drop2.item] = drop2.amount
    end
  end

end

For checking the result:

p h
p drop1.amount
p drop2.amount


Other option, if it is viable for you, let the class do the job defining it like this:

class Drop
  attr_accessor :item, :price, :amount

  def initialize(item:'no_name', price: 0, amount: 0)
    @item = item
    @price = price
    @amount = amount
    @counts = 0
    @increment = amount
  end

  def count!
    @counts += 1
    @amount += @increment if @counts > 1
  end

end

Then store the instances in an array:

drops = []
drops << Drop.new(item: 'item1', price: 2247, amount: 1)
drops << Drop.new(item: 'item2', price: 4401, amount: 60)

Run the random picking sampling the drops array:

10.times do |x|
  drops.sample.count!
end

Check the result:

drops.each do |drop|
  puts "#{drop.item} - #{drop.amount} - #{drop.price}"
end

You can also define a reset method which restores the amount and the counts to the original value:

def reset
  @amount = @increment
  @count = 0
end
Related