Roman numerals to ints

[Christopher Benson-Manica]

Inspired by a thread on clc++, I decided to try it out...

#include <stdio.h>
#include <stdlib.h>

int main( int argc, char *argv[] )
{
int i;
int result=0;
int this;
int last=0;

if( argc != 2 ) {
printf( "No string specified\n" );
return( EXIT_FAILURE );
}
for( i=0 ; argv[1] != '\0' ; i++ ) {
switch( argv[1] ) {
case 'M':
this=1000;
break;
case 'D':
this=500;
break;
case 'C':
this=100;
break;
case 'L':
this=50;
break;
case 'X':
this=10;
break;
case 'V':
this=5;
break;
case 'I':
this=1;
break;
default:
printf( "Bad character %c\n", argv[1] );
return( EXIT_FAILURE );
}
result+=this;
if( this > last ) {
result-=2*last;
}
last=this;
}
printf( "Result is %d\n", result );
return( EXIT_SUCCESS );
}

This seems to work (and this time I didn't forget my header files ).
Questions:

A) Is it 100% legal, ANSI C?
B) Is there a more efficient way to implement it?

 

[Tom Zych]

Christopher Benson-Manica wrote:

Looks pretty good overall. A few ways to make it shorter:

for( i=0 ; argv[1] != '\0' ; i++ ) {
switch( argv[1] ) {

char *p;

for (p = argv[1]; *p; p++) {
switch (*p) {

Pointers are your friend, in C.

case 'M':
this=1000;
break;
case 'D':
this=500;
break;

This way uses 1/3 as many lines and is easier to read besides:
case 'M': this = 1000; break;
case 'D': this = 500; break;

You could also use a table for the conversion, something like:

char letters[] = "MDCLXVI";
char values[] = {1000, 500, 100, 50, 10, 5, 1};

And use strchr(), but you'd have to subtract pointers and mess
around a bit. I think the switch statement is cleaner, really.

Now (getting silly here), if you had to convert *lots* of roman
numerals to arabic *really fast*, you could build a table of 256
values, where table['M'] = 1000, etc, and invalid characters were
zero...

 

[Neil Cerutti]

Inspired by a thread on clc++, I decided to try it out...

#include <stdio.h>
#include <stdlib.h>

int main( int argc, char *argv[] )
{
int i;
int result=0;
int this;
int last=0;

if( argc != 2 ) {
printf( "No string specified\n" );

A usage note is good here, e.g. roman: usage roman
[roman numeral]. It makes the error message a little more
helpful.

return( EXIT_FAILURE );
}
for( i=0 ; argv[1] != '\0' ; i++ ) {
switch( argv[1] ) {
case 'M':
this=1000;
break;
case 'D':
this=500;
break;
case 'C':
this=100;
break;
case 'L':
this=50;
break;
case 'X':
this=10;
break;
case 'V':
this=5;
break;
case 'I':
this=1;
break;
default:
printf( "Bad character %c\n", argv[1] );
return( EXIT_FAILURE );
}
result+=this;
if( this > last ) {
result-=2*last;
}
last=this;
}
printf( "Result is %d\n", result );
return( EXIT_SUCCESS );
}

This seems to work (and this time I didn't forget my header
files ). Questions:

A) Is it 100% legal, ANSI C?

If you mean: "Is it 100% in accordance with the langauge
described in the C standard everybody is talking about?", then
yes.

I have no idea if writing such a program as the above is actually
legal. Consult a lawyer for that. ;-)

B) Is there a more efficient way to implement it?

I think it's efficient enough.

C) Does it work?

It seems to. The best way to test this program is with another
program that converts base 10 to roman numerals, or obtain a huge
table of conversions. You can then write a quick script to
convert a few million numbers there and back again, and thus have
more confidence in your program.

It would be nice if it also accepted lower case roman digits.

Finally, it may work for valid roman numerals, but it doesn't
provide useful diagnostics for nonsense combinations of roman
digits like "VIM" or "IXI".

 

[The Real OS/2 Guy]

Inspired by a thread on clc++, I decided to try it out...

#include <stdio.h>
#include <stdlib.h>

int main( int argc, char *argv[] )
{
int i;
int result=0;
int this;
int last=0;

if( argc != 2 ) {
printf( "No string specified\n" );
return( EXIT_FAILURE );
}
for( i=0 ; argv[1] != '\0' ; i++ ) {
switch( argv[1] ) {
case 'M':
this=1000;
break;
case 'D':
this=500;
break;
case 'C':
this=100;
break;
case 'L':
this=50;
break;
case 'X':
this=10;
break;
case 'V':
this=5;
break;
case 'I':
this=1;
break;
default:
printf( "Bad character %c\n", argv[1] );
return( EXIT_FAILURE );
}
result+=this;
if( this > last ) {
result-=2*last;
}
last=this;
}
printf( "Result is %d\n", result );
return( EXIT_SUCCESS );
}

This seems to work (and this time I didn't forget my header files ).
Questions:

A) Is it 100% legal, ANSI C?
Yes.

B) Is there a more efficient way to implement it?

/* you may use this table to convert dec to roman too */

struct {
char rom;
long dec;
} rom_dec[] = {
{ 'I', 1 },
{ 'V', 5; },
{ 'X', 10 },
{ 'L', 50 },
{ 'C', 100 },
{ 'D', 500 },
{ 'M', 1000},
{ 0, 0 } /* end of list, bail out, unknown letter */
};

char *r = argv[1];
char *d = rom_dec;
long result = 0;
long last = 0;

for (; *r, r++) {
for (d = rom_dec; d->rom && *r != d->rom; d++) ; /* for body is
empty! */
if (!d->rom) {
printf("unable to convert %c\n", *r);
return EXIT_FAILTURE;
}
result += d->dec;
if (d->dec > last)
result -= 2*last;
last = this;
}

Not tested yet, but should work

 

[Richard Heathfield]

The Real OS/2 Guy wrote:

for (; *r, r++) {
for (d = rom_dec; d->rom && *r != d->rom; d++) ; /* for body is
empty! */
if (!d->rom) {
printf("unable to convert %c\n", *r);
return EXIT_FAILTURE;
}
result += d->dec;
if (d->dec > last)
result -= 2*last;
last = this;
}

Not tested yet, but should work

On the contrary, the compiler will issue a diagnostic for this code, and
almost certainly refuse to provide a binary.

 

[Christopher Benson-Manica]

for (; *r, r++) {
for (d = rom_dec; d->rom && *r != d->rom; d++) ; /* for body is
empty! */
if (!d->rom) {
printf("unable to convert %c\n", *r);
return EXIT_FAILTURE;
}

Is looping through a table more efficient than a switch statement? (the code
certainly looks cleaner)

 

[The Real OS/2 Guy]

The Real OS/2 Guy wrote:



On the contrary, the compiler will issue a diagnostic for this code, and
almost certainly refuse to provide a binary.

Why should it? Don't cry that it is not a complete function!

 

[Jirka Klaue]

^
....

Why should it?

Why do you ask?

Jirka

 

[Kevin D. Quitt]

Why should it? Don't cry that it is not a complete function!

gcc foo.c
foo.c:6: parse error before ';' token
foo.c:15: `argv' undeclared here (not in a function)
foo.c:16: warning: initialization from incompatible pointer type
foo.c:20: parse error before "for"
foo.c:20: conflicting types for `r'
foo.c:15: previous declaration of `r'
foo.c:20: conflicting types for `r'
foo.c:20: previous declaration of `r'
foo.c:20: parse error before '++' token
foo.c:30: conflicting types for `last'
foo.c:18: previous declaration of `last'
foo.c:30: `this' undeclared here (not in a function)
foo.c:30: warning: data definition has no type or storage class
foo.c:31: parse error before '}' token

 

[Neil Cerutti]

Is looping through a table more efficient than a switch
statement? (the code certainly looks cleaner)

I don't think there's much benefit to it over a big switch unless
the table allows lookup faster than linear time (O(n)).

You could do that with a table big enough to hold every valid
character value, or with a table sorted by char value, on which
you can use binary search.

A linear search through the table, as above, probably isn't
better than what you've got now.

 

[Charles Richmond]

The Real OS/2 Guy wrote:



On the contrary, the compiler will issue a diagnostic for this code, and
almost certainly refuse to provide a binary.

When I was in college, there was an interesting algorithm
for converting Roman numerals to ints. The jist of it is:
First convert the V's and I's at the end of the number to
a decimal digit. Then strip off the V's and I's from the
end of the string. Use a "translate" function to change
all X's to I's, all L's to V's, all C's to X's, etc.
Basically, this divides the Roman numeral by ten. Repeat
until the string containing the Roman numeral is empty...
(Of course, you have to accumulate the decimal digits
in the right order...)

 

[The Real OS/2 Guy]

I don't think there's much benefit to it over a big switch unless
the table allows lookup faster than linear time (O(n)).

It's more flexible. You have no change to the code when you have to
inert a new entry in the list. O.k. you may have to write a new
function to handle that entry - but not a bit to change existing code.
That reduces the possibility of errors.

You could do that with a table big enough to hold every valid
character value, or with a table sorted by char value, on which
you can use binary search.

You can sort the table by the probable frequency an entry may occure.
Again no codechande.

A linear search through the table, as above, probably isn't
better than what you've got now.

Any change of code increases the possibility to buid new bugs in.
Making code that is short, easy to maintenance and flexible increases
the maintencability, reduces problems and increases the useability.

 

[Neil Cerutti]

It's more flexible. You have no change to the code when you
have to inert a new entry in the list. O.k. you may have to
write a new function to handle that entry - but not a bit to
change existing code. That reduces the possibility of errors.

That does not speak to the efficiency of the resulting code.

You can sort the table by the probable frequency an entry may
occure. Again no codechande.


Any change of code increases the possibility to buid new bugs
in. Making code that is short, easy to maintenance and flexible
increases the maintencability, reduces problems and increases
the useability.

Making code more maintainable and usable does make the code more
maintainable and usable. I agree! ;-)

 

[MikeyD]

Is looping through a table more efficient than a switch

It's more flexible. You have no change to the code when you have to
inert a new entry in the list. O.k. you may have to write a new
function to handle that entry - but not a bit to change existing code.
That reduces the possibility of errors.

Dude there's not exactly going to be an extra letter added to the roman
numbering system