# Unexpected behavior, it this a bug?

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]]

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.

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

It’s never a bug.

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

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.

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

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?

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

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.

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.

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