6
\$\begingroup\$

I'm exploring ruby by writing a code that checks the sum of all prime numbers under two million. As I am still in the process of learning ruby, I'm unfamiliar with all the standards and best practices. What I have so far works, but I am certain it can be improved. Where can I improve my code for both readability and speed?

def is_prime(n)
    range = n
    i = 2

    while i < range do
        return false if (n % i == 0)
        range = n / i
        i += 1
    end
    true
end

sum = 2
(3..2000000).each do |num|
    if (is_prime(num))
        #puts "#{num}"
        sum += num
    end
end

puts "#{sum}"
\$\endgroup\$
3
  • 1
    \$\begingroup\$ As for speed, you should try using athosteles sieve. Also, it's unnecessary to go up to n to decide if it's prime or not, going up to the square root is enough \$\endgroup\$
    – juvian
    Oct 30, 2015 at 14:52
  • \$\begingroup\$ Conceptually, primality is an attribute that belongs to an integer. Idiomatic Ruby is pretty hardcore about being object-oriented, so your primality test should be a method of the Integer class unless you have a good reason for it not to be. if num.prime?, not if (is_prime(num)). \$\endgroup\$
    – ezrast
    Oct 31, 2015 at 1:17
  • \$\begingroup\$ And, as always, notice that except for 2 all primes are odd. So you only need to check 50% of the candidates. Here: i += 2 with i starting at 3. \$\endgroup\$ Oct 31, 2015 at 21:58

1 Answer 1

6
\$\begingroup\$

Ruby favours Functional Programming, let me show you an example.

def prime?(n)
  limit = Math.sqrt(n).to_i + 1
  (2..limit).all? { |i| n % i != 0 }
end

This improves readability as the code makes use of all? to implicitly loop (Instead you looped explicitly) and is faster as the limit is the sqrt of n and not n and all? short-circuits (returns as soon as one condition is false).


Also the second part would benefit from FP:

puts (2..2000000).select{ |n| prime?(n) }.inject(:+)

In general, in Ruby explicit looping is the exception and not the norm, as there are so many built-ins and shortcuts to make use of.

Each function I made use of here is listed at The Ruby Enumerable Documentation and I warmly suggest studying in detail that page, it will take a bit of time, but the time you will save in the future by using it instead of reinventing the wheel each time will be more than worth it.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ I believe that should be limit = Math.sqrt(n).to_i (no +1). Otherwise the sum comes out two off. \$\endgroup\$
    – Shelby115
    Oct 30, 2015 at 19:34
  • \$\begingroup\$ @Shelby115 3 should be 2 in the range \$\endgroup\$
    – Caridorc
    Oct 30, 2015 at 23:51
  • \$\begingroup\$ @FilipBartuzi Cool trick! I knew &: but not :& \$\endgroup\$
    – Caridorc
    Oct 31, 2015 at 15:51
  • \$\begingroup\$ @Caridorc actually I was very wrong. My codesnippet called Fixnum#even? not function defined in main scope \$\endgroup\$ Oct 31, 2015 at 15:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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