HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Prime number calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberprimecalculator

Problem

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:

  • Adherence to Ruby standards



  • Is it done in the Ruby way (the way a Rubyist would have coded it)?



  • 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

Solution

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.

Code Snippets

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

Context

StackExchange Code Review Q#5792, answer score: 3

Revisions (0)

No revisions yet.