1
\$\begingroup\$

The problem my code solves is listed below. I know my code can be improved, it takes a while to run when bigger arrays are entered as input.

"Problem - Given an array of integers, sum consecutive even numbers and consecutive odd numbers. Repeat the process while it can be done and return the length of the final array."

def sum_groups(arr)

  z = arr.size
  x = []
  i = 0

  until i == z
    arr = arr.chunk{|x| x.even?}.map{|x, y| y}.map{|x| x.inject(&:+)}
    x << arr
    i += 1
  end 

  x[-1].size

end

An example input -

For arr = [2, 1, 2, 2, 6, 5, 0, 2, 0, 5, 5, 7, 7, 4, 3, 3, 9]

The result should be 6.

[2, 1, 10, 5, 30, 15] - Value of numbers in final array.

The length of final array is 6

Any review and help would be appreciated to make this code more efficient.

\$\endgroup\$
1
\$\begingroup\$

The whole thing about sums is misdirection; you don't need to sum anything, since all you need to output is the final length of the array. So what you want to know is how many consecutive terms there are of a given parity, not what their sum is.

The trick is that only an odd number of odd terms will produce an odd sum; anything else will produce an even sum. So you can skip calculating the actual sum, because by knowing the parity and number of terms, you'll know if that sum's going to be odd or even.

Secondly, you can make it recursive instead of loop-based.

Here's my take:

def count_chunks_recursively(array)
  # get our consecutive chunks
  chunks = array.chunk(&:odd?).to_a

  # if there are as many chunks as there are elements, we're done
  return array.size if chunks.size == array.size

  # otherwise, map the chunks to 0 or 1, depending on whether they
  # sum up to something even or something odd
  mapped = chunks.map { |odd, terms| odd && terms.size.odd? ? 1 : 0 }

  # ... and repeat the process recursively
  count_chunks_recursively(mapped)
end

As for your current code:

  • You very, very rarely have to use plain loops in Ruby; there's almost always something in Enumerable or Array that you can use instead.

  • x[-1].size would be more conventionally written as x.last.size

  • You use the a method reference when producing the sum – inject(&:+) – but you don't use the same technique for the other parts of the same line. I.e. this

    arr.chunk{|x| x.even?}.map{|x, y| y}.map{|x| x.inject(&:+)}
    

    could just be this:

    arr.chunk(&:even?).map(&:last).map { |x| x.inject(&:+) }
    

    or in Ruby 2.4+ which has a built-in sum method:

    arr.chunk(&:even?).map(&:last).map(&:sum)
    

    Sidenote: inject (specifically) actually doesn't need the method reference, but can also take a plain symbol: inject(:+). But for consistency's sake, I'd stick to using the &.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.