I am new to Ruby (1 year as hobby project on weekends) and solved a code challenge and it was not as good as they wanted. I want to know how i could make it better and how to make the structure better. In summary, how would a mid or senior ruby developer do this challenge?
This is the task:
Write a program that prints a multiplication table of primes numbers. The program should take an argument from the command line that specifies the amount of prime numbers to generate and print out a multiplication table for these prime numbers.
An example of the way the application may run:
executable_script_name --count 10
An example of the output (using the terminaltable gem):
| 2 3 5 7 11 13 17 19 23 29 + 2 | 4 6 10 14 22 26 34 38 46 58 3 | 6 9 15 21 33 39 51 57 69 87 5 | 10 15 25 35 55 65 85 95 115 145 7 | 14 21 35 49 77 91 119 133 161 203 11 | 22 33 55 77 121 143 187 209 253 319 13 | 26 39 65 91 143 169 221 247 299 377 17 | 34 51 85 119 187 221 289 323 391 493 19 | 38 57 95 133 209 247 323 361 437 551 23 | 46 69 115 161 253 299 391 437 529 667 29 | 58 87 145 203 319 377 493 551 667 841
Notes
- Consider code readability/complexity
- Consider SOLID principles, but do not overengineer
- Consider extensibility
- Feel free to use any library or gem in both implementation and tests, but please write your own code for the prime number generator.
- Consider how you can prove the correctness of your application
- Write it in Ruby
Below starts my code challenge:
require 'rubygems'
require 'bundler/setup'
require 'optparse'
require './calculate_prime'
# Just parsing the options here and
# initialize the Calculater to present
# the table of results.
options = {}
OptionParser.new do |opts|
opts.banner = 'Usage: prime_multiply.rb [options]'
opts.on('-v', '--count N', Integer, 'Select amount of primes') do |v|
options[:count] = v
end
end.parse!
primes = CalculatePrime.new(options[:count])
puts primes.present
This code above is just for run terminal, check arguments and and call the actual program and return the table.
require 'terminal-table'
require 'optparse'
require './array_addons'
# This program multiplies all primes and
# add them to a multidimensional array and
# present them as a table in the terminal
# This class handle all the logic in the program.
# It use a addition to the Array class in array_addons
# to make it easier to understand the code.
class CalculatePrime
attr_reader :primes, :counted
FIXNUM_MAX = (2**(0.size * 8 - 2) - 1)
# initialize the class and create number of primes picked
def initialize(count)
@primes = get_prime_numbers(count)
multiply
end
# gets amount of primes from Prime class
def get_prime_numbers(size)
primes = []
return [] if size < 1
(2..FIXNUM_MAX).each do |num|
# want to break it when got all asked primes so it will
# not go for infinite
break if primes.size >= size
# check if num is odd by dividenum and add it
primes.push(num) if (2..num - 1).all? { |dividenum| num % dividenum > 0 }
end
primes
end
# multiply the primes with it's values with
# the multiply_by method in my own array addition.
def multiply
@counted = []
@primes.each do |n|
# using unshift to add a extra of n to the beginning
# of the array to show it nice in the table later
@counted.push(@primes.multiply_by(n).unshift(n))
end
end
# return the data that was multiplied in a table
def present
# unshifting a empty string to make a empty column
# between header and childcolumns
table = Terminal::Table.new headings: @primes.unshift(''), rows: @counted
table
end
end
If you wonder why i am not using Ruby's Prime class it is because they wanted me to make the prime method myself in Ruby.
And then it is just the added method to array:
# This class just add a collect method for multiply_by
# to make it easier to understand the code.
class Array
def multiply_by(x)
collect { |n| n * x }
end
end
I think the biggest no-no in my code is the FIXNUM_MAX
constant. But how could I have done it in a better way?
And PS. They didn't even let me explain my reasons why i did some of the code and not even a thank you for the challenge so I feel i did a really bad challenge. I also have rspec tests and can add them if you wonder that could be the reason why they not even let me discuss it.
My Gemfile is:
source 'http://rubygems.org'
# terminal stuff
gem 'terminal-table'
# testing
gem 'rspec'
# code quality checks
gem 'rubocop'
EDIT: Got answer now after some time:
- TDD and the Red/Green/Refactor cycle
- Seperating the concerns of CalculatePrime class as it had way to many responsibilities because it was responsible for calculating the prime numbers, multiplying the numbers and printing the table each one of these could have made into their own classes with one responsibility, I would recommend reading 'Practical Object Oriented design in Ruby' by Sandi Metz as this has some good examples of this.
I guess they was very picky and strict. TDD is mostly because of lack of experience and the other thing well, is also a little bit of lack of experience but also I felt I didn't want too small classes either. But they didn't let me explain that. These stuff are also easy to learn.