Integer to Roman Numeral conversion

Question

As a little standard exercise, I wrote an integer to roman numerals converter in F03. The result looks already neat enough, for my standards, but I wonder whether it could be made snappier without sacrificing clarity. For folks not familiar with the simple syntax of modern Fortran, I suggest the following quick modern Fortran tutorial.

module mRomanStrings
 implicit none
 private
 character(len=*), parameter :: RM_CHARS = "IVXLCDMvxlcdm_"
 integer , parameter :: MAX_ORDER = len(RM_CHARS)/2-1
 character(len=4) :: ROMAN_STRING(0:9,0:MAX_ORDER)
 logical :: mRomanStringsIsNotInitialized = .TRUE.
 public :: IntegerToRoman
contains
 function IntegerToRoman(i) result(rm)
 integer , intent(in) :: i
 character(len=:), allocatable :: rm
 integer :: n
 if( mRomanStringsIsNotInitialized ) call mRomanSring_init()
 rm = " "
 do n = min(MAX_ORDER,int(log(dble(i))/log(10.d0)+1.d-10)), 0, -1
 rm = trim(rm)//trim(ROMAN_STRING(mod(i/10**n,10),n))
 enddo
 end function IntegerToRoman
 subroutine mRomanSring_init()
 character :: a0, a1, a2, a3
 integer :: n
 do n = 0, MAX_ORDER
 a0 = " "
 a1 = RM_CHARS(2*n+1:2*n+1)
 a2 = RM_CHARS(2*n+2:2*n+2)
 a3 = RM_CHARS(2*n+3:2*n+3)
 ROMAN_STRING(0:9,n) = [&
 a0//a0//a0//a0, a1//a0//a0//a0, a1//a1//a0//a0, a1//a1//a1//a0, a1//a2//a0//a0, &
 a2//a0//a0//a0, a2//a1//a0//a0, a2//a1//a1//a0, a2//a1//a1//a1, a1//a3//a0//a0 ]
 enddo
 mRomanStringsIsNotInitialized = .FALSE.
 end subroutine mRomanSring_init
 
end module mRomanStrings
program ConvertIntegersToRoman
 use, intrinsic :: iso_fortran_env
 use mRomanStrings
 implicit none
 integer :: i
 do i=1,10000
 write(OUTPUT_UNIT,"(i6,x,a)") i, IntegerToRoman(i)
 enddo
end program ConvertIntegersToRoman

• mcocdawc: There is an error in your the initialization routine. 2 * MAX_ORDER + 3 > size(RM_CHARS)
• mcocdawc: If you compile with gfortran -g -Wall -Wextra -Warray-temporaries -Wconversion -fimplicit-none -fbacktrace -ffree-line-length-0 -fcheck=all -ffpe-trap=zero,overflow,underflow -finit-real=nan You get automatically warned about boundary violations.

Answer 1

It is overall good code, but there are of course some points to improve.

Objectively wrong and severe:

1. Always compile with as much debug options as possible with your compiler. It detects errors such as

   2 * MAX_ORDER + 3 == 2 * (size(RM_CHARS) / 2 - 1) + 3 > size(RM_CHARS)
   

   in your initialization loop.

2. The function expects a number that is larger than zero and smaller than 10^6. This is nowhere documented nor checked.

Small things that could be made better:

1. There are unnecessary many trim calls when you build up the string. If you only append trimmed strings, you know that the result is trimmed.

2. There is log10 for the decadic logarithm.

3. It helps sometimes to introduce additional variable names for self documentation.

4. I would recommend to use the words magnitude, exponent, and mantissa in the code. This is more self-explaining than "order".

5. It is not necessary to start from Zero for the first dimension of ROMAN_STRING

6. I would never name a logical variable with its negated form. You can always use .not. and it quickly becomes hard to read if you doubly negate it. The name could be shorter in addition.

   mRomanStringsIsNotInitialized == .not. mRomanStringInitialized
   .not. mRomanStringsIsNotInitialized == mRomanStringInitialized
   

7. I would use floor instead of int. It is a common error to use int instead of nint so it is nice to explicitly say floor.

Architecture:

At the moment you have runtime checks in your function to ensure initialization of ROMAN_STRING. This is not necessary since ROMAN_STRING could be made a compile time constant (with parameter). If you do this then your function does not depend anymore on a global variable that can be mutated at run time, can be made pure and is probably a tiny bit faster.

Opinion based:

I would add more whitespace around your operators and after commata and use an indentation of at least 4 spaces. You can format Fortran code pretty much according to the PEP8 guidelines of python, to achieve convincing results. (Especially since a lot of people in scientific computing use both languages.)

Everything together leads to:

 module mRomanStrings
 
 implicit none
 private
 integer , parameter :: MAX_MAGNITUDE = 5 
 character(len=4), parameter :: ROMAN_STRING(9, 0 : MAX_MAGNITUDE) = & 
 reshape([character(4) :: &
 'I', 'II', 'III', 'IV', 'V', 'VI', 'VII', 'VIII', 'IX', &
 'X', 'XX', 'XXX', 'XL', 'L', 'LX', 'LXX', 'LXXX', 'XC', &
 'C', 'CC', 'CCC', 'CD', 'D', 'DC', 'DCC', 'DCCC', 'CM', &
 'M', 'MM', 'MMM', 'Mv', 'v', 'vM', 'vMM', 'vMMM', 'Mx', &
 'x', 'xx', 'xxx', 'xl', 'l', 'lx', 'lxx', 'lxxx', 'xc', &
 'c', 'cc', 'ccc', 'cd', 'd', 'dc', 'dcc', 'dccc', 'cm'], &
 [9, MAX_MAGNITUDE + 1]) 
 public :: IntegerToRoman
 
 contains
 
 !> Expects an integer 1 <= n <= (10^6 - 1) and returns
 !> the Roman number literal as string.
 pure function IntegerToRoman(n) result(rm)
 integer, intent(in) :: n
 character(len=:), allocatable :: rm
 integer :: largest_magnitude, mag, mantissa
 if (n <= 0) error stop
 
 largest_magnitude = floor(log10(dble(n)))
 if (largest_magnitude > MAX_MAGNITUDE) error stop
 
 rm = ""
 do mag = largest_magnitude, 0, -1
 mantissa = mod(n / 10**mag, 10) 
 if (mantissa > 0) then
 rm = rm // trim(ROMAN_STRING(mantissa, mag))
 end if
 end do
 end function IntegerToRoman
 
 end module mRomanStrings
 
 program ConvertIntegersToRoman
 use, intrinsic :: iso_fortran_env, only: OUTPUT_UNIT
 use mRomanStrings
 implicit none
 integer :: i
 do i = 1, 99999
 write(OUTPUT_UNIT,"(i6,x,a)") I, IntegerToRoman(i)
 end do
 end program ConvertIntegersToRoman

• Luca Argenti: Thanks for the detailed and informative feedback mcocdawc. Not sure about the four-space indentation: I like the default in emacs. By the way, it looks like julia should be a more natural companion to people in the fortran community than python. Any thoughts?
• Vladimir F Героям слава: When I see floor instead of int I am starting to look what is going on with negative numbers and get confused when it is in fact nothing.
• mcocdawc: @LucaArgenti I worked once with Julia for a numeric task when it had version 0.6. Even then I liked it, and I guess they have improved the things that I did not like. (It was fast after JIT compilation, but the first execution of a function was really slow. Since you could not easily precompile functions, it was slower than python if you only executed functions once.) If I started a new numerics project now, I would definitely consider Julia, but since our number crunching tasks rely on existing Fortran code with more than 10^6 lines of code you just don't rewrite it.
• mcocdawc: For postprocessing/plotting I have good workflow with numpy/pandas/matplotlib and speed does not matter, so I won't change it. I assume it is the case for a lot of colleagues. Nevertheless I am happy to have at least written a short serious program with Julia, because their multiple dispatch paradigm is really nice. It definitely improved my code to use a bit less OOP and a bit more overloaded but free functions. (free as in not a method) PS: With vim you write much better Fortran code. ;-)
• mcocdawc: @VladimirF I am sorry, but I don't really get your argument floor(-3.5) == -4 is IMHO not nothing.