Unexpected behavior, it this a bug?

Ruby
1

The following code produces totally unexpected/undesired behavior.

Is it a bug?

The following method adds the prime factors of two numbers which are in
the
form [prime,exponent]. It performs the operation correctly but for some
reason it changes the elements of the first array, when I would expect
it to have no effect on the input arrays.

  1. Is this correct or a bug (it occurs in all Ruby version <= 2.3.1)?

  2. If this is not a bug, how do I write this so as not to change the
    input arrays?

def product_factors(a,b)
factors = (a + b).sort!
prod_factors = []
until factors.empty?
factor = factors.shift
factor[1] += factors.shift[1] if !factors.empty? && factor[0] ==
factors.first[0]
prod_factors << factor
end
prod_factors
end

a = [[2, 1], [3, 1], [5, 2], [7, 2], [11, 2], [41, 1], [271, 1], [9091,
1]]
b = [[2, 5], [3, 2], [5, 2], [7, 3], [11, 2], [13, 2], [37, 1], [41, 1]]

product_factors a, b
=> [[2, 6], [3, 3], [5, 4], [7, 5], [11, 4], [13, 2], [37, 1], [41, 2],
[271, 1], [9091, 1]]

a
=> [[2, 6], [3, 3], [5, 4], [7, 5], [11, 4], [41, 2], [271, 1], [9091,
1]]
b
=> [[2, 5], [3, 2], [5, 2], [7, 3], [11, 2], [13, 2], [37, 1], [41, 1]]

2

When I run it, I get changes in both (a & b) arrays. (ruby2.0.0)

I’m not a super guru, but I suspect that what’s actually happening is
the arrays within your array are getting modified.

So, a isn’t getting modified, but the arrays with a are.

factors[0] and a[0] are the same object (initially). You then shift out
of factors and into factor. So, variable factor now refers to the same
thing as a[0].

You’ll probably need to use dup (factor = factors.shift.dup) instead.

3
  1. Is this correct or a bug (it occurs in all Ruby version <= 2.3.1)?

It’s never a bug. :smiley:

  1. If this is not a bug, how do I write this so as not to change the
    input arrays?

The easiest way to do this is to not give the function the things you
don’t want changed, so start with something like:

def product_factors(factors)
end

Which you can call with:
product((a + b).sort)

There’s also need to use sort in place, ie. sort! Also you need to be
very careful when using methods like Array#shift.

Then you can think about using something like Enumerable#each_with_index
to get the next index/ element. It’s hard for me to understand what you
are doing, but it looks like you want:

def multiply(factors)
consumed_next = false
result = Array.new
factors.each_with_index do |factor, index|
unless (index + 1) == factors.length
next_factor = factors[index + 1]
if next_factor.first == factor.first
result << [ factor.first, next_factor.last + factor.last ]
consumed_next = true
else
result << factor unless consumed_next
consumed_next = false
end
else
result << factor
end
end
result
end

As you can see, this is a horrible mess. You probably want to change
this into an object, because you have some state, represented by the
flag consumed_next.

4

As noted, this is not a bug. You are mutating the internal structures of
the arrays you are passing in.

To avoid this, I usually just build a new data structure, instead of
trying to dup everything that could be mutated.

I think I would use #reduce and a hash to solve this.

def product_factors(a, b)
(a + b).reduce({}) { |memo, (prime, exp)|
memo[prime] = (memo[prime] || 0) + exp
memo
}.sort
end

Anyway, I hope this helps :slight_smile:

5

So, aehm, this initializes memo as a hash object, then it iterates over
element in the array, the first value in the sub-array is assigned to
prime, the second to exp and then if memo already has a key with prime,
exp is added onto the existing value other wise the prime key is added
and assigned exp!

Awesome! Thanks for this. I didn’t know that you could do that. What is
happening with the (exp, prime)? How is that working?

I see that (a, b) = [1, 2] is the same as a, b = 1, 2, so this is an
example of multiple assignment?

6

Jabari Z. wrote in post #1183258:

The following code produces totally unexpected/undesired behavior.

Is it a bug?

def product_factors(a,b)
factors = (a + b).sort!
‘factors’ contains elements of a and b, in a different order
prod_factors = []
until factors.empty?
factor = factors.shift
‘factor’ point to first element of factors
factor[1] += factors.shift[1] if !factors.empty? && factor[0] ==
factors.first[0]
'factor[1] is mofified, so element is modified , so element of a is
modified!!!
prod_factors << factor
end
prod_factors
end

def product_factors(a,b)
factors = (a + b).sort!
prod_factors = []
until factors.empty?
e = factors.shift
if !factors.empty? && e.first == factors.first.first
e=[e.first, e.last+factors.shift.last ] # here we create e new
tuple
end
prod_factors << e
end
prod_factors
end

Or, in your code, you can clone factor :
factor=factors.shift.clone

7

Ruby supports destructuring arrays. That’s all I’m doing with prime and
exp. It’s sort of like multiple assignment :). For me, that’s easier to
understand then indexing into an array.

The assignment part is a little dense, but it is basically this:

  • Get the existing exponent value for prime. If there isn’t one, assume
    zero.
  • Add the current exponent value to the existing exponent value.
  • Store this new sum for the value of prime.
8

Thanks everyone for your suggestions.
The easiest fix was to replace

factor = factors.shift

with

factor = factors.shift.dup (or …shift.clone, they both work)

Still is was surprising for me (I learned something) that
passing in arrays that are combined into a new named array
can result in the input arrays to be modified without being
operated on by name. I learned something new about Ruby’s
object model.

Thank all, again.

9

Hi Ryan, sure is easier to understand. Much appreciated!