Prime number calculator

Question

I am starting to learn Ruby.

This is pretty much the first thing I have coded. It's very simple; it's a prime number calculator. Since this is my first Ruby code, I would like some review on the following:

1. Adherence to Ruby standards
2. Is it done in the Ruby way (the way a Rubyist would have coded it)?
3. Adherence to Ruby naming conventions

What I am not looking for is a review on the prime number algorithm. I know there are more efficient ones out there.

---

File: prime_numbers.rb

class PrimeNumber
 def is_prime_kata(number)
 if number == 1 then return false end 
 max = Math.sqrt(number)
 (2..max).any? do |i| 
 if number % i == 0 then return false end
 end
 true
 end
end

---

File: prime_numbers_test.rb

require 'test/unit'
require 'set'
require_relative 'prime_numbers.rb'
class PrimeNumberTest < Test::Unit::TestCase
 def test_is_prime
 primeNumbers = [2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97]
 primeNumber = PrimeNumber.new
 for i in primeNumbers
 result = primeNumber.is_prime_kata(i)
 assert_equal(true, result)
 end
 primeNumbersSet = Set.new(primeNumbers)
 allNumbersSet = Set.new(1..100) 
 nonPrimeNumbersSet = allNumbersSet - primeNumbersSet
 for i in nonPrimeNumbersSet
 result = primeNumber.is_prime_kata(i)
 assert_equal(false, result)
 end
 end 
end

• tokland: first advice: 2-space indentation.

Answer 1

Minor things. I will leave your actual questions to proper "Rubyists". :)

defensive programming

I would change the first condition to

return false if number <= 1

0 is not a prime number, and negative numbers aren't either.
Maybe I would also adapt the test to check these cases.

brevity

if <condition> then <statement> end

can be written

<statement> if <condition>

, so you could write

return false if number == 1

and

return false if number % i == 0

• Gilles: Thank you, this is exactly the kind of input I was looking for.
• ANeves: I understand. But please note that I am just a beginner in Ruby as well. (Just an older beginner.)

Answer 2

Aiming at your questions, your naming conventions are good. However, in your test you tend to switch between camelCase and words_separated_by_underscores. I would suggest sticking to one or the other (we tend to use underscores). Your algorithm looks good and taking ANeves suggestions will make it look more "Ruby"ish.

However, I am going to suggest a much more efficient way of generating your prime numbers and none prime numbers in your Test. Instead of typing each prime number between 1 and 100 by hand into an Array, you could use the Array#Select method. It takes in an Enumerator, runs a given block against each item passed in, and returns an Array of only the items that return true.

prime_numbers = (1..100).select {|num| num.prime?}

This will given you the same Array you got by typing it all in manually. You can do the same for the none prime numbers.

non_prime_numbers = (1..100).select {|num| num unless num.prime?}

This'll return num only if it is not prime.

It'll save you a lot of typing and it will look more "Ruby"ish.

Answer 3

I'd write:

class PrimeNumber
 def is_prime_kata(number)
 return false if number == 1 
 max = Math.sqrt(number) 
 (2..max).all? { |x| number % x != 0 }
 end
end

Answer 4

Per your request, I suggest:

File: prime_numbers.rb

class PrimeNumbers
 def self.prime_kata? number
 n=number.floor
 return false if n < 2
 max=Math.sqrt(n).floor
 (2..max).none?{|k| 0==n % k}
 end
end

File: prime_numbers_test.rb

require 'mathn'
require 'test/unit'
require_relative 'prime_numbers'
class PrimeNumbersTest < Test::Unit::TestCase
 LIMIT=100
 def test_is_prime_or_not
 primes=Prime.each(LIMIT).to_a
 non_primes = (0.. LIMIT).to_a - primes
 non_primes.each{|k| assert ! (PrimeNumbers.prime_kata? k)}
 primes. each{|k| assert PrimeNumbers.prime_kata? k }
 end
end