9
\$\begingroup\$

Could someone please point out the error(s) in the given code? It was downvoted on Stack Overflow without any explanation, but it seems to be working fine for me:

int value(char roman)
{
   switch(roman)
   {
        case 'I':return 1;
        case 'V':return 5;
        case 'X':return 10;
        case 'L':return 50;
        case 'C':return 100;
        case 'D':return 500;
        case 'M':return 1000;
   }
}

int getdec(const string& input)
{
  int sum=0; char prev='%';
  for(int i=(input.length()-1); i>=0; i--)
  {
    if(value(input[i])<sum && (input[i]!=prev))
    {       sum -= value(input[i]);
            prev = input[i];
    }
    else
    {
            sum += value(input[i]);
            prev = input[i];
    }
  }
  return sum;
}

This was the output received from the code:

I = 1
II = 2
III = 3
IV = 4
V = 5
VI = 6
VII = 7
VIII = 8
IX = 9
X = 10
XI = 11
XII = 12
XIII = 13
XIV = 14
XV = 15
XVI = 16
XVII = 17
XVIII = 18
XIX = 19
XX = 20
XXI = 21
XXII = 22
XXIII = 23
XXIV = 24
XXV = 25
XXVI = 26
XXVII = 27
XXVIII = 28
XXIX = 29
XXX = 30
XXXI = 31
XXXII = 32
XXXIII = 33
XXXIV = 34
XXXV = 35
XXXVI = 36
XXXVII = 37
XXXVIII = 38
XXXIX = 39
XL = 40
MMMMCMXCIX = 4999
CM = 900
XC = 90

Improve this question
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Roman Numeral converter in 855 chars :-) codegolf.stackexchange.com/questions/797/… \$\endgroup\$
    – Martin York
    Feb 19, 2013 at 2:57
  • \$\begingroup\$ I think IIX would break your code... \$\endgroup\$
    – David Anderson
    May 9, 2013 at 2:38
  • \$\begingroup\$ Your code isn't complete: it has no main(), nor any definition of string. Is that supposed to be std::string from #include <string>? Post something that actually compiles. \$\endgroup\$
    – Toby Speight
    Oct 14, 2016 at 9:57
  • \$\begingroup\$ Googling Marcus Caelius tombstone reveals that he was the leader of legion XIIX. Ancient Romans were essentially pragmatic people: The reason they would not write 40 as XXXX is that it was shorter to write CO \$\endgroup\$
    – Pierre
    Apr 14, 2020 at 10:25

3 Answers 3

Reset to default
6
\$\begingroup\$

If you have only valid Roman numbers you could use a simpler algorithms. (i.e IIV is invalid )

  • The symbols "I", "X", "C", and "M" can be repeated three times in succession, but no more. (They may appear more than three times if they appear non-sequentially, such as XXXIX.) "V", "L", and "D" can never be repeated. A common exception to this is the use of IIII on clocks; see below.
  • "I" can be subtracted from "V" and "X" only. "X" can be subtracted from "L" and "C" only. "C" can be subtracted from "D" and "M" only. "V", "L", and "D" can never be subtracted
  • Only one small-value symbol may be subtracted from any large-value symbol.

Just (string-)replace IV by IIII, IX by VIIII and so on. Afterwards you just have to sum the numbers from left to right.

Improve this answer
\$\endgroup\$
0
5
\$\begingroup\$

There is a G++ compile warning (g++ -Wall):

roman.cpp: In function ‘int value(char)’:
roman.cpp:18:1: warning: control reaches end of non-void function [-Wreturn-type]

It should handle invalid inputs too. (Furthermore, it returns 9 for IIIIIIIII.)

Improve this answer
\$\endgroup\$
2
  • \$\begingroup\$ Thanks. So validation checks are needed but otherwise the logic is correct? \$\endgroup\$
    – user1071840
    Feb 19, 2013 at 0:25
  • \$\begingroup\$ @user1071840: To be honest, I don't know. I've checked it with 5-10 different Roman numerals and it worked fine but I don't know too much about the Roman numerals-decimal numbers conversion algorithms. \$\endgroup\$
    – palacsint
    Feb 19, 2013 at 0:41
0
\$\begingroup\$

I didn't try to run it but it seems to me that this algorithm won't handle "Double substractive" forms. Forms like XIIX instead of XVIII. How described on wiki – it's not typical roman number usage, but it was used in the past so the program should probably be prepared for it.

Improve this answer
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Generally, XIIX is not considered to be a valid roman number. Each number can only be written in one and only one way with roman numerals. \$\endgroup\$
    – Simon Forsberg
    Oct 13, 2016 at 16:11
  • \$\begingroup\$ I am not expert but according to wiki there is nothing like a "valid" roman number. ... The "standard" forms described above reflect typical modern usage rather than a universally accepted convention. Usage in ancient Rome varied greatly and remained inconsistent in medieval and modern times... \$\endgroup\$
    – bujon
    Dec 8, 2016 at 13:41

Not the answer you're looking for? Browse other questions tagged or ask your own question.