-2

I made up this problem for myself. This code prints integer numbers up to n in Romans. The code works fine. Is there any way to make it shorter and more efficient? Also, point out any mistakes made.

#include <stdio.h>

void roman(int num)
    {
        printf("%d = ", num);
        while(num)
            {
                if(num>=1000)
                    {
                        num=num-1000;
                        printf("M");
                        continue;
                    }
                if(num>=900)
                    {
                        num=num-900;
                        printf("CM");
                        continue;
                    }
                if(num>=500)
                    {
                        num=num-500;
                        printf("D");
                        continue;
                    }
                if(num>=400)
                    {
                        num=num-400;
                        printf("CD");
                        continue;
                    }
                if(num>=100)
                    {
                        num=num-100;
                        printf("C");
                        continue;
                    }
                if(num>=90)
                    {
                        num=num-90;
                        printf("XC");
                        continue;
                    }
                if(num>=50)
                    {
                        num=num-50;
                        printf("L");
                        continue;
                    }
                if(num>=40)
                    {
                        num=num-40;
                        printf("XL");
                        continue;
                    }
                if(num>=10)
                    {
                        num=num-10;
                        printf("X");
                        continue;
                    }
                if(num>=9)
                    {
                        num=num-9;
                        printf("IX");
                        continue;
                    }
                if(num>=5)
                    {
                        num=num-5;
                        printf("V");
                        continue;
                    }
                if(num>=4)
                    {
                        num=num-4;
                        printf("IV");
                        continue;
                    }
                if(num>=1)
                    {
                        num=num-1;
                        printf("I");
                        continue;
                    }
            }
        printf("\n");
    }

void main()
    {
        int scan;
        printf("Print Roman numbers till: ");
        scanf("%d", &scan);
        for(int i=1;i<=scan;i++)
            roman(i);
    }

The code works fine. Is there any way to make it shorter and more efficient? Point out mistakes I've made, if any.

pexeixv
  • 89
  • 7
  • 6
    You should post this to [codereview.se] instead. – Guy Incognito Jun 15 '20 at 15:48
  • Not sure what you mean by "more efficient", but a stylistic point that would almost certainly be that would be to use `putchar` (or `fputc`, or `putc`) rather than `printf` to print a single character. – William Pursell Jun 15 '20 at 15:58

1 Answers1

1

You can make your code shorter and more efficient by storing a map of all the numbers, as such:

int nums[] = {1, 4, 5, 9, 10, 40, 50, 90, 100, 400, 500, 900, 1000};
string letters[] = {"I","IV","V","IX","X","XL","L","XC","C","CD","D","CM","M"};

Then, you can just iterate from the reverse end of the nums[] list to construct your number:

void roman(int num){
    size_t i = sizeof(nums)/sizeof(int); // length of nums[]
    while(num > 0){
        int rep = num/nums[i]; // number of times we reduce num by
        num = num%nums[i];
        for(int j = 0; j < rep; ++j){
            cout << letters[i];
        }
    }
}

Another way you could boost the efficiency is by using binary search to search through nums[] instead of just iterating from the end. Both of these approaches will work efficiently.

Telescope
  • 2,068
  • 1
  • 5
  • 22