It is overall good code, but there are of course some points to improve.
Objectively wrong and severe:
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.
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:
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.
log10 for the decadic logarithm.
It helps sometimes to introduce additional variable names for self documentation.
I would recommend to use the words magnitude, exponent, and mantissa in the code. This is more self-explaining than "order".
It is not necessary to start from Zero for the first dimension of
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
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
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
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.
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:
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
!> 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 function IntegerToRoman
end module mRomanStrings
use, intrinsic :: iso_fortran_env, only: OUTPUT_UNIT
integer :: i
do i = 1, 99999
write(OUTPUT_UNIT,"(i6,x,a)") I, IntegerToRoman(i)
end program ConvertIntegersToRoman
2 * MAX_ORDER + 3 > size(RM_CHARS)\$\endgroup\$
gfortran -g -Wall -Wextra -Warray-temporaries -Wconversion -fimplicit-none -fbacktrace -ffree-line-length-0 -fcheck=all -ffpe-trap=zero,overflow,underflow -finit-real=nanYou get automatically warned about boundary violations. \$\endgroup\$