5
\$\begingroup\$

Write a function lucky_sevens?(numbers), which takes in an array of integers and returns true if any three consecutive elements sum to 7.

lucky_sevens?([2,1,5,1,0]) == true # => 1 + 5 + 1 == 7
lucky_sevens?([0,-2,1,8]) == true # => -2 + 1 + 8 == 7
lucky_sevens?([7,7,7,7]) == false
lucky_sevens?([3,4,3,4]) == false

Make sure your code correctly checks for edge cases (i.e. the first and last elements of the array).

While my code does seem to work (I did some test cases), I know it can be improved. There's no reason to have double puts and return statements. I'm also wondering if there are general improvements to the syntax itself that can make it much more readable.

def lucky_sevens?(numbers)
  i = 0
  while i < numbers.length - 2

    if numbers[i] + numbers[i + 1] + numbers[i + 2] == 7
        puts true
        return true
    end
    i+=1
    end 
    puts false
    return false
end
\$\endgroup\$
4
\$\begingroup\$

Your code, is way too imperative to be idiomatic Ruby.

The only reasonable improvement is re-writing it from scratch:

def lucky_sevens?(numbers)
  groups_of_three(numbers)
    .any? {|group| group.inject(:+) == 7}
end

Knowing that inject(:+) means sum, this reads like English. Implementing groups_of_three is left as an exercise for the reader.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Could the downvoter explain why? I am always happy to receive constructive criticism :) \$\endgroup\$ – Caridorc Oct 26 '15 at 14:16
4
\$\begingroup\$

Looping using an index is not idiomatic in Ruby. Looping using #each and Enumerable methods based on #each are much more common. This is a perfect opportunity to use Enumerable#each_cons(3).

def lucky_sevens?(numbers)
  numbers.each_cons(3) { |group| return true if group.reduce(:+) == 7 }
  false
end

The problem doesn't ask you to print anything, so don't.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Writing groups_of_three was left as an exercise for the reader :o, showing the right built-in right away in my opinion seems to encourage laziness... \$\endgroup\$ – Caridorc Oct 20 '15 at 21:49
  • 1
    \$\begingroup\$ Don't use my return true if …. @Caridorc's any? is much better. \$\endgroup\$ – 200_success Oct 20 '15 at 21:51
  • \$\begingroup\$ @200_success, In fairness, your version is more performant as it will return immediately upon finding a success, whereas the any? version requires all groups of three to be calculated, even if, say, the first 3 numbers in the array summed to 7 \$\endgroup\$ – Jonah Oct 21 '15 at 17:40
  • \$\begingroup\$ Also, depending on your context, the false return value at the end may be unnecessary, since each_cons returns nil, which will evaluate to false. \$\endgroup\$ – Jonah Oct 21 '15 at 17:44
  • 1
    \$\begingroup\$ @Jonah Enumeration#any? does short-circuit. You can see that [3,2,1,0].each_cons(3).any? { |group| 1 / group.min } succeeds, while [2,1,0].each_cons(3).any? { |group| 1 / group.min } crashes. \$\endgroup\$ – 200_success Oct 21 '15 at 17:46

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.