# Group array by sums

I have the following code that takes an array, and groups it by the sums of its elements:

class Array
def in_sums_of size, &block
block = -> el { el } unless block
container = []

each_with_index.inject([]) do |tmp, group|
element, idx = group
value = block.call(element)

sum = tmp.inject(0) { |sum,x| sum + block.call(x) }

if sum + value <= size
tmp << element
else
container << tmp if tmp.any?
tmp = [element]
end
container << tmp if idx == length - 1

tmp
end

container
end
end

[].in_sums_of(0)
# => []

[1].in_sums_of(1)
# => [[1]]

[1,1].in_sums_of(1)
# => [[1], [1]]

[1,1].in_sums_of(2)
# => [[1, 1]]

[3,2,1].in_sums_of(3)
# => [[3], [2, 1]]

[3,3].in_sums_of(3)
# => [[3], [3]]

[4,3,1].in_sums_of(3)
# => [[4], [3], [1]]

[{v: 1},{v: 2},{v: 2},{v: 3}].in_sums_of(3) do |el|
el[:v]
end
# => [[{:v=>1}, {:v=>2}], [{:v=>2}], [{:v=>3}]]


It takes an array and a limit, and puts elements into sub arrays, as long as there is space left, which is limited by size var (3 in the example above).

The code works, and is already in production, but I was curious if this is the best approach. I haven't found a Rails helper or Ruby method that would help with this approach. So any ideas on this?

EDIT I've updated the code and added some examples.

• Could you please add several more examples of inputs and outputs of your methods, including some edge cases. I'm confident it's possible to refactor (since initializing variables and temp variables can almost always be avoided Ruby), but I'm not confident I understand what the method is supposed to do. Also, what is the result supposed to be if no block is included? – Dan Kohn Jul 4 '17 at 14:34
• I've updated the code. Do you still see some edge cases that I've missed? – 23tux Jul 5 '17 at 9:14

## 2 Answers

You can use chunk, generating a new "chunk id" each time your accumulating sum overflows its max. chunk abstracts away all the details of the inner arrays, so you don't have to worry about when to create a new one, or manually push items onto them. This greatly simplifies the code:

class Array
def in_sums_of(size, &block)
block ||= -> x { x }
sum, id  = 0, 0

chunk do |x|
sum += block.call(x)
sum = block.call(x) and id += 1 if sum > size
id
end.map {|x| x.last}

end
end


As an aside, I'd prefer to create this as a utility method, rather than monkey-patching the Array class, but that change doesn't affect the code much.

• That is brilliant! Exactly "the ruby way" I was looking for. Thanks! – 23tux Jul 6 '17 at 6:38
• You may be able to use chunk_while (Ruby >=2.3 I believe) which would eliminate the final map. – Mark Thomas Jul 7 '17 at 19:08

I would consider simplifying this:

block = -> el { el } unless block
container = []
each_with_index.inject([]) do |tmp, group|
element, idx = group
value = block.call(element)
sum = tmp.inject(0) { |sum,x| sum + block.call(x) }


to:

container = []
each_with_index.inject([]) do |tmp, (element, index)|
value = block_given? ? yield(element) : element
sum = tmp.sum(&block)


Its arguable about what is better but I would write this as:

class Array
def in_sums_of size

sum = 0
each_with_object([]) do |element, container|
value = block_given? ? yield(element) : element
if container.empty? || sum + value > size
container << [ element ]
sum = value
else
container.last << element
sum += value
end
end

end
end