patternrubyMinor
Prime number calculator
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:
What I am not looking for is a review on the prime number algorithm. I know there are more efficient ones out there.
File:
File:
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.rbclass 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
endFile:
prime_numbers_test.rbrequire '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
endSolution
Aiming at your questions, your naming conventions are good. However, in your test you tend to switch between
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.
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.
This'll return
It'll save you a lot of typing and it will look more "Ruby"ish.
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.