I have this program that tries to convert from arabic to roman numbers to arabic numbers and it compiles without problems, but for certain values it gives the correct answer but for other cases it does not. For example:

11 is wrong (an X remains to the left of the Xs) 12 is wrong (an X remains to the left of the Xs)

20 is good

22 is wrong (2 X left over)

30 is good

32 is wrong (2 X left over)

40 is good

45 is bad (there is an XL left) 48 is wrong (an X remains before the second L)

50 is good

51 gives bad (left over an L) 58 is bad (left over an L)

60 is good

char *a2roman (int valor, char *c1, char *c2, char *c3);

int main (void)
{
    int ArabicNumber = 1;
    int result;
    char roman[15] = "";

    do
    {
        printf ("Enter an integer in the range 1 to 3000: \n\t");

        scanf ("%d", &ArabicNumber);
    }
    while ((ArabicNumber < 1) || (ArabicNumber > 3000));

    if ((ArabicNumber <= 3000) && (ArabicNumber >= 1000))
    {
        result = ArabicNumber / 1000;
        strcat (roman, a2roman(result, "M", " ", " "));
        ArabicNumber -= (result * 1000);
    }

    if ((ArabicNumber < 1000) && (ArabicNumber >= 100))
    {
        result = ArabicNumber / 100;
        strcat (roman, a2roman(result, "C", "D", "M"));
        ArabicNumber -= (result * 100);
    }

    if ((ArabicNumber < 100) && (ArabicNumber >= 10))
    {
        result = ArabicNumber / 10;
        strcat (roman, a2roman(result, "X", "L", "C"));
        ArabicNumber -= (result * 10);
    }

    if ((ArabicNumber < 10) && (ArabicNumber >= 1))
    {
        strcat (roman, a2roman(ArabicNumber, "I", "V", "X"));
    }

    printf ("The Roman numeral is: \n\t%s\n\n", roman);
    printf ("\t\t...Press any key to finish.");
    getch();

    return 0;
}

char *a2roman (int value, char *c1, char *c2, char *c3)
{
    int i;
    static char rRoman[15] = "";

    /* Si "valor" = 1, 2, 3 */
    if ((value >= 1) && (value <= 3))
    {
        for (i = 0; i < value; i  )
            strcat (rRoman, c1);
    }

    /* Si "valor" = 5, 6, 7, 8 */
    if ((value >= 5) && (value <= 8))
    {
        strcat (rRoman, c2);

        for (i = 0; i < (value - 5); i  )
            strcat (rRoman, c1);
    }

    /* Si "valor" = 4 */
    if (value == 4)
    {
        strcat (rRoman, c1);
        strcat (rRoman, c2);
    }

    /* Si "valor" = 9 */
    if (value == 9)
    {
        strcat (rRoman, c1);
        strcat (rRoman, c3);
    }

    return (rRoman);
}

CodePudding user response：

The static buffer inside a2roman builds up the complete result but each time it is called from main the partial result is also concatenated to the buffer that is printed out so you get duplicates if a2roman is called more than once.

One solution would be to use the return value from a2roman directly after the full result has been constructed by making roman a pointer and assigning the return value from a2roman to it each time a2roman is called like:

char* roman = NULL;
...
roman = a2roman(result, "M", " ", " ");
...
roman = a2roman(result, "C", "D", "M");
...
printf ("The Roman numeral is: \n\t%s\n\n", roman);

But if you ever wanted to convert more than one number you'd still have a problem because of the static buffer. You could clear it between numbers if you had a pointer to it, like *roman = 0; but that still feels messy to me.

A different solution is to pass in the buffer you want the number to be constructed in.

The following example shows how you could do that and also cleans up a little bit of the logic used to break the number into digits. What you had works but I figured I might as well show an alternate method and save a few lines.

#include <stdio.h>
#include <string.h>

void addDigit(char* out, int digit, const char* c1, const char* c2, const char* c3)
{
    if ((digit >= 1) && (digit <= 3))
    {
        for (int i = 0; i < digit; i  )
        {
            strcat(out, c1);
        }
    }

    if ((digit >= 5) && (digit <= 8))
    {
        strcat(out, c2);

        for (int i = 0; i < (digit - 5); i  )
        {
            strcat(out, c1);
        }
    }

    if (digit == 4)
    {
        strcat(out, c1);
        strcat(out, c2);
    }

    if (digit == 9)
    {
        strcat(out, c1);
        strcat(out, c3);
    }
}

void convertNumber(char* out, int num)
{
    if ((num <= 3000) && (num >= 1000))
    {
        addDigit(out, num / 1000, "M", " ", " ");
        num %= 1000;
    }

    if ((num < 1000) && (num >= 100))
    {
        addDigit(out, num / 100, "C", "D", "M");
        num %= 100;
    }

    if ((num < 100) && (num >= 10))
    {
        addDigit(out, num / 10, "X", "L", "C");
        num %= 10;
    }

    if ((num < 10) && (num >= 1))
    {
        addDigit(out, num, "I", "V", "X");
    }
}

int main(void)
{
    for (int i = 1; i < 100; i  = 3)
    {
        char buf[32] = { 0 };
        convertNumber(buf, i);
        printf("M = %s\n", i, buf);

    }
    for (int i = 101; i < 3000; i  = 101)
    {
        char buf[32] = { 0 };
        convertNumber(buf, i);
        printf("M = %s\n", i, buf);
    }
    return 0;
}

Online Example

Output:

   1 = I
   4 = IV
   7 = VII
  10 = X
  13 = XIII
  16 = XVI
  19 = XIX
  22 = XXII
  25 = XXV
  28 = XXVIII
  31 = XXXI
  34 = XXXIV
  37 = XXXVII
  40 = XL
  43 = XLIII
  46 = XLVI
  49 = XLIX
  52 = LII
  55 = LV
  58 = LVIII
  61 = LXI
  64 = LXIV
  67 = LXVII
  70 = LXX
  73 = LXXIII
  76 = LXXVI
  79 = LXXIX
  82 = LXXXII
  85 = LXXXV
  88 = LXXXVIII
  91 = XCI
  94 = XCIV
  97 = XCVII
 101 = CI
 202 = CCII
 303 = CCCIII
 404 = CDIV
 505 = DV
 606 = DCVI
 707 = DCCVII
 808 = DCCCVIII
 909 = CMIX
1010 = MX
1111 = MCXI
1212 = MCCXII
1313 = MCCCXIII
1414 = MCDXIV
1515 = MDXV
1616 = MDCXVI
1717 = MDCCXVII
1818 = MDCCCXVIII
1919 = MCMXIX
2020 = MMXX
2121 = MMCXXI
2222 = MMCCXXII
2323 = MMCCCXXIII
2424 = MMCDXXIV
2525 = MMDXXV
2626 = MMDCXXVI
2727 = MMDCCXXVII
2828 = MMDCCCXXVIII
2929 = MMCMXXIX