Robert Klemme
4/22/2006 9:56:00 AM
adamw@ratio.co.uk wrote:
> Hi
> I'm new to Ruby and programming and currently trying to write a
> program converting Roman numerals to integers. I've done the
> following so far (number validation check still not good) and stuck
> on the step when I need to delete a specified string from the
> beginning (left side) of some string and keep the rest. I've tried
> many String class methods but none of them seems to meet my needs.
>
> def roman_to_integer number
> number = number.to_s
> result = 0
> roman = [ 'M', 'CM', 'D', 'CD', 'C', 'XC', 'L', 'XL', 'X', 'IX', 'V',
> 'IV', 'I' ]
> arab = [ 1000, 900, 500, 400, 100, 90, 50, 40, 10, 9, 5, 4, 1 ]
> if (number.capitalize).include?('M' || 'D' || 'C' || 'L' || 'X' ||
> 'V' || 'I')
> n = 0
> while n < 13
> while number.include?(roman[n])
> number[n].delete(roman[n]) <- THIS IS THE LINE
> result = result + arab[n]
> end
> n += 1
> end
> print 'It equals: ' + result.to_s
> else
> puts 'Please enter a valid Roman numeral'
> end
> end
> roman_to_integer 'MCmLxxVI'
>
> Please help
> Cheers
> Adam
You can use []= to remove stuff from a string:
>> s="12345"
=> "12345"
>> s[0,2]=''
=> ""
>> s
=> "345"
>> s[-2..-1]='x'
=> "x"
>> s
=> "3x"
Here are some ideas for other improvements you can do to your code:
- your test does not work as you expect, notice this
>> 'M' || 'D' || 'C' || 'L' || 'X' || 'V' || 'I'
=> "M"
So your code basically just does
if number.capitalize.include? 'M'
(Explanation: || is the boolean operator and as everything apart from false
and nil is treated as true it short circuits on the first element and
returns that.)
- you can make the sanity check with a regexp plus using an exception is
cleaner, e.g.
raise ArgumentError, "not a roman: #{number}" unless /^[mdclxvi]+$/ =~
number
(This one won't catch all illegal roman numbers, for example "IIII" goes
undetected, but it's a start and it does what you were actually trying to
accomplish with your test.)
- make roman and arab constants, so you do not recreate them on each method
invocation
- use #each_with_index for easier iteration or even Array#zip for iterating
in lock step, but:
- probably it's even better to merge roman and arab into a Hash, because
mapping of roman numbers to arab numbers is what you basically do
- in the next version you probably will be creating a class RomanNumber
that inherits Integer and implements +(), -(), coerce() etc.
Hope that helps.
Kind regards
robert