Is my code unelegant?

Ruby
1

Is the attached code too messy to be considered “good”?

I can’t figure out if I created too many variables.

I’m just now learning Ruby, and I just can’t figure out if that’s bad
code.

2

I should add that the point of this program is to have a user enter a
number between 1 and 3000 and it will convert it to Old Roman Numerals.

3

One immediate modification would be to replace the final two lines of
your roman_numerals() method with:

(‘M’ * number_of_m) + (‘D’ * number_of_d) + (‘C’ * number_of_c) + (‘L’

  • number_of_l) + (‘X’ * number_of_x) + (‘V’ * number_of_v) + (‘I’ *
    number_of_i)

since the method will return that without the need for an explicit
return statement.

You’ll notice that there is no puts in this, either. Your method
should return a string. Creating the roman numeral version of the
given number is a separate concern to actually printing that number.
You’re better off then using puts roman_numerals(argument).

Just looking at it, I see nothing which restricts the input to between
1 and 3000.

4

On Sep 5, 4:49 pm, Mark W. [email protected] wrote:

Is the attached code too messy to be considered “good”?

I can’t figure out if I created too many variables.

I’m just now learning Ruby, and I just can’t figure out if that’s bad
code.

Attachments:http://www.ruby-forum.com/attachment/5007/romannumerals1.rb

–
Posted viahttp://www.ruby-forum.com/.

def old_roman n
[1000,500,100,50,10,5,1].
map{|d| quot, n = n.divmod(d); quot }.
zip( %w(M D C L X V I) ).
map{|n,c| c * n}.join
end

puts "Enter a number and I’ll convert " +
“it to the older Roman Numerals!”
puts old_roman( gets.to_i )