1
\$\begingroup\$

I have written a program to convert a decimal to a Roman numeral. I am wondering if my algorithm is okay or not. If it's not okay, I want suggestions on improving my code. This code only works for inputs up to 5000. I found this question in a code cracker competition at my college.

#include <iostream>
using namespace std;
int main()
{
    int num=0;
    int user;
    int temp=0;
    string ans[4];
    int mod=10;
    int n[5];
    string r1[]={"0","I","II","III","VI","V","VI","VII","VIII","IX" };
    string r2[]={"x","XX","XXX","XL","L","LX","LXX","LXXX","XC"};
    string r3[]={"C","CC","CCC","CD","D","DC","DCC","DCCC","CM"};
    string r4[]={"M","MM","MMM","MMMM"};

    cout<<"Enter Number"<<endl;
    cin>>user;

    for(int i=0;i<4;i++)
{
    num=user%mod;
    user=user-num;
    n[3-i]=num;
    mod=mod*10;
}

for(int i=0;i<4;i++)
{
    if(n[i]>999)
    {
        temp=n[i]/1000;
        ans[i]=r4[temp-1];
    }
    if(n[i]>99 && n[i]<1000)
    {
        temp=n[i]/100;
    ans[i]=r3[temp-1];
    }
    if(n[i]>9&& n[i]<100)
    {
    temp=n[i]/10;
    ans[i]=r2[temp-1];
    }
        if(n[i]>0 && n[i]<10)
    {
    temp=n[i];
    ans[i]=r1[temp];
    }
    }
    for(int i=0;i<4;i++)
    {
    cout<<ans[i];
    }
    return 0;
    }
\$\endgroup\$
  • 2
    \$\begingroup\$ 1) Do not add C tag for C++ code. 2) If your code works then it is OT here. SO is no code review site. If it does not work, please state what you have done and what specific problem you have and what you have do to solve yourself. \$\endgroup\$ – Olaf Sep 19 '15 at 14:40
  • \$\begingroup\$ ok.I didn't know that..... \$\endgroup\$ – Æ Ām Anup Sep 19 '15 at 14:45
  • \$\begingroup\$ @ÆĀmAnup So does your code really work? All I can tell you is that it is doing a lot more than it should to convert from an integer to Roman numerals. \$\endgroup\$ – PaulMcKenzie Sep 19 '15 at 14:53
  • \$\begingroup\$ You may be more likely to get suggestions about your algorithm if you describe how it works in words. \$\endgroup\$ – גלעד ברקן Sep 19 '15 at 15:08
  • \$\begingroup\$ @PaulMcKenzie yup my code is really working... and i also know my code is not better.. do u have any suggetions? \$\endgroup\$ – Æ Ām Anup Sep 19 '15 at 15:31
2
\$\begingroup\$

This code has a bug as 14 and 16 give the same output, while the Roman representation for them should surely be different:

  • 14 -> XVI
  • 16 -> XVI
\$\endgroup\$
1
\$\begingroup\$

The algorithm does not seem to implement putting a Roman number together using power of ten letters "I, X, C, M" and half power of ten letters "V, L, D", and relies on pre-coded conversions in the string array. I'm not saying do it this way but I'll share a javascript function to show how this might done (consider it pseudo code for C++):

function toRoman( num)
{   var str = "";
    var xstr;
    var xdigs = String("IVXLCDMvxlcdm_______");
    var xdig;   // roman digit
    var dig;    // digit

    if( num <= 0 || num >= 4000000) // out of range error
    { return "Out of range";
    }
    for( var i = 0; num > 0; i+=2)
    {   xstr = "";
        xdig = xdigs.charAt(i);  // a power of ten Roman letter
        dig = num % 10;
        num -= dig;
        num /= 10;

        if( (dig+1) % 5 == 0)
        {   xstr = xdig; // prepend subtrahend
            ++dig;
        }
        if( dig == 10)
        {   xstr += xdigs.charAt(i+2); // use next power of 10
        }
        else
        {   if( dig >= 5) // use fives
            {   xstr += xdigs.charAt(i+1); // half of next higher power of ten
                dig -=5;
            }
            while( dig-- > 0)
            {   xstr += xdig;         // postpend addend
            }
        }
        str = xstr + str;          // build leftwards
    }
    return str;
}

Note that Roman numbers use a bar over a letter to multiply it by 1000, while this code uses lowercase to indicate multiplied by 1000. Enjoy!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy