EltonTom - 1 year ago 103

Ruby Question

The question I'm working on is:

Find which sum of squared factors are a perfect square given a specific range.

So if the range was (1..10) you would get each number's factors (all factors for 1, all factors for 2, all factors for 3 ect..) Square those factors, then add them together. Finally check if that sum is a perfect square.

I am stuck on refactoring/optimization because my solution is too slow.

Here is what I came up with:

`def list_squared(m, n)`

ans = []

range = (m..n)

range.each do |i|

factors = (1..i).select { |j| i % j == 0 }

squares = factors.map { |k| k ** 2 }

sum = squares.inject { |sum,x| sum + x }

if sum == Math.sqrt(sum).floor ** 2

all = []

all += [i, sum]

ans << all

end

end

ans

end

This is an example of what I would put in the method:

`list_squared(1, 250)`

And then the desired output would be an array of arrays with each array containing the number whose sum of squared factors was a perfect square and the sum of those squared factors:

`[[1, 1], [42, 2500], [246, 84100]]`

Answer Source

I would start by introducing some helper methods (`factors`

and `square?`

) to make your code more readable.

Furthermore I would reduce the number of ranges and arrays to improve memory usage.

```
require 'prime'
def factors(number)
[1].tap do |factors|
primes = number.prime_division.flat_map { |p, e| Array.new(e, p) }
(1..primes.size).each do |i|
primes.combination(i).each do |combination|
factor = combination.inject(:*)
factors << factor unless factors.include?(factor)
end
end
end
end
def square?(number)
square = Math.sqrt(number)
square == square.floor
end
def list_squared(m, n)
(m..n).map do |number|
sum = factors(number).inject { |sum, x| sum + x ** 2 }
[number, sum] if square?(sum)
end.compact
end
list_squared(1, 250)
```

A benchmark with a narrow range (up to `250`

) shows only a minor improvement:

```
require 'benchmark'
n = 1_000
Benchmark.bmbm(15) do |x|
x.report("original_list_squared :") { n.times do; original_list_squared(1, 250); end }
x.report("improved_list_squared :") { n.times do; improved_list_squared(1, 250); end }
end
# Rehearsal -----------------------------------------------------------
# original_list_squared : 2.720000 0.010000 2.730000 ( 2.741434)
# improved_list_squared : 2.590000 0.000000 2.590000 ( 2.604415)
# -------------------------------------------------- total: 5.320000sec
# user system total real
# original_list_squared : 2.710000 0.000000 2.710000 ( 2.721530)
# improved_list_squared : 2.620000 0.010000 2.630000 ( 2.638833)
```

But a benchmark with a wider range (up to `10000`

) shows a much better performance than the original implementation:

```
require 'benchmark'
n = 10
Benchmark.bmbm(15) do |x|
x.report("original_list_squared :") { n.times do; original_list_squared(1, 10000); end }
x.report("improved_list_squared :") { n.times do; improved_list_squared(1, 10000); end }
end
# Rehearsal -----------------------------------------------------------
# original_list_squared : 36.400000 0.160000 36.560000 ( 36.860889)
# improved_list_squared : 2.530000 0.000000 2.530000 ( 2.540743)
# ------------------------------------------------- total: 39.090000sec
# user system total real
# original_list_squared : 36.370000 0.120000 36.490000 ( 36.594130)
# improved_list_squared : 2.560000 0.010000 2.570000 ( 2.581622)
```

tl;dr: The bigger the `N`

the better performs my code compared to the original implementation...