8
\$\begingroup\$

I'm doing an archived MOOC at the moment which introduces Ruby programming. There are some assingments, yet no solutions and since the MOOC is over no one will correct those (+ the forum is down)

I'm totally new to Ruby (background in Java) and asking for some hints on my solution to the following task (my solution under the task):

Define a method sum which takes an array of integers as an argument and returns the sum of its elements. For an empty array it should return zero.

def sum(array)
  array[0] = array.pop + array[0];
  sum(array) until array.length == 1
  array.first
end

Define a method max_2_sum which takes an array of integers as an argument and returns the sum of its two largest elements. For an empty array it should return zero. For an array with just one element, it should return that element.

def max_2_sum(a)
  case a.length
    when 0
      return 0
    when 1
      return a.first
    else
      return (a.sort!.pop)+(a.sort!.pop)
  end
end

I think this is pretty verbose, yet I do not know how to solve that better


Define a method sum_to_n? which takes an array of integers and an additional integer, n, as arguments and returns true if any two distinct elements in the array of integers sum to n. An empty array or single element array should both return false.

def sum_to_n?(array, val)
  cartesian = array.product(array).select! { |c| c[0]+c[1] == val }
  not cartesian.empty?
end

I like this :)

\$\endgroup\$
  • \$\begingroup\$ Do you have any particular concerns with your code? It's a bit unclear what it is that you're asking here. \$\endgroup\$ – RubberDuck May 20 '15 at 12:28
  • 1
    \$\begingroup\$ I'm not sure if this is "the ruby way" - never worked with a dynamic language before & not so expirenced with programming at all \$\endgroup\$ – ruby000 May 20 '15 at 12:30
  • \$\begingroup\$ Better do not call methods right after !, because ! methods can return nil. \$\endgroup\$ – Nakilon May 20 '15 at 23:04
8
\$\begingroup\$

Ruby can be an extremely concise language, where your programs are almost already written for you:

def sum(array)
    array.inject(0, :+)
end

Where the docs say:

inject { |memo, obj| block } → obj

Combines all elements of enum by applying a binary operation, specified by a block or a symbol that names a method or operator.

In fact reading the docs can go a long way in shortening your programs.


Reading the docs for Array tells us that sorting is built-in and last (the twin of first) are too, also, we just wrote a handy sum function.

def max_2_sum(array)
    sum(array.sort.last(2))
end

This is slower than needed, that is O log n instead of O n but it may be optimized iff it becomes a performance bottleneck (improbable).


Your last function is indeed nice, but the assignment was not necessary and you did not use the handy built-in any?

def sum_to_n?(array, val)
    array.product(array).any? {|couple| sum(couple) == val}
end
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks, this is really helpful. Ruby has a lot of sophisticated methods, thanks for pointig me there. Yet your max_2_sum does not return "0" if the array is empty :) \$\endgroup\$ – ruby000 May 20 '15 at 13:58
  • 1
    \$\begingroup\$ You should use 0 as the initial memo for inject: [].inject(0, :+) # => 0 whereas [].inject(:+) # => nil. \$\endgroup\$ – toro2k May 20 '15 at 15:41
  • \$\begingroup\$ @toro2k done :) \$\endgroup\$ – Caridorc May 20 '15 at 18:12
  • 1
    \$\begingroup\$ Also note that you shouldn't use the ! versions of functions when you're doing an inline operation with them -- they only return the new value if it was changed, in most cases. \$\endgroup\$ – Nic Hartley May 20 '15 at 19:12
  • \$\begingroup\$ In v.2.2 a new form of Enumerable#max returns a specified number of elements of the receiver in non-increasing order; for example, [1,4,2,5].max(2) #=> [5,4]. This allows you to write: def max_2_sum(arr); arr.max(2).reduce(0,:+); end. max_2_sum([]) #=> 0, max_2_sum([1]) #=> 1, max_2_sum([1,2]) #=> 3, max_2_sum([1,2,3]) #=> 5. \$\endgroup\$ – Cary Swoveland May 29 '15 at 3:01
3
\$\begingroup\$

Instead of providing solutions, I'll review the code by stating what not to do.

Your sum function, while technically correct, has three shortcomings:

  • It's inefficient, in that it manipulates the array more than necessary.
  • By calling Array#pop, it mutilates the input array in the process. While the problem statement doesn't explicitly forbid you from modifying the array, it is good practice to refrain from doing so.
  • It uses recursion. What's more, it does so in a weird way that also involves an until loop, and it also discards the result of the recursive call. If you did want to write it recursively (I'm not saying it's a good idea), it should look like

    def sum(array)
      array.empty? ? 0 : array.last + sum(array[0, array.size - 1])
    end
    

Your max_2_sum also mutilates the input array in two ways: sorting it in place and removing elements from it.

The (a.sort!.pop) + (a.sort!.pop) line is a bit deceptive, in that the first a and the second a refer to the same array, but in different states. There is no sense in sorting the same array twice — you can just sort it once and take the last two elements.


Your sum_to_n? solution is incorrect, in that it fails to take into account the word distinct in the problem statement. A proper solution would probably involve Array#combination.

Also, sum_to_n?([3], 6) crashes with NoMethodError: undefined method `empty?' for nil:NilClass, due to the way Array#select! works:

If changes were made, it will return self, otherwise it returns nil.

As a rule of thumb, methods whose name ends with ! tend to be a bit "weird", and you should probably read the docs thoroughly. In many cases, especially if you're a beginner, the version without ! is preferable.

\$\endgroup\$
  • \$\begingroup\$ "Instead of providing solutions..." Thanks for the reminder of what Code Review is for. \$\endgroup\$ – jcm May 21 '15 at 9:51
1
\$\begingroup\$

Define a method sum which takes an array of integers as an argument and returns the sum of its elements. For an empty array it should return zero.

The accepted answer's array.inject(:+) returns nil, not 0, when given an empty array. A simple change fixes it:

def sum(array)
  array.inject(0, :+)
end

Define a method max_2_sum which takes an array of integers as an argument and returns the sum of its two largest elements. For an empty array it should return zero. For an array with just one element, it should return that element.

Given the fix to the sum method above the accepted answer should work.

def max_2_sum(array)
  sum(array.sort.last(2))
end

Define a method sum_to_n? which takes an array of integers and an additional integer, n, as arguments and returns true if any two distinct elements in the array of integers sum to n. An empty array or single element array should both return false.

By 'distinct elements' do you mean elements with different values, or different indices? For example, in the array [1, 1, 2], can 1 be paired with 1?

\$\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.