A c++ question

[shaidul]

Hello,

Can you please help me correct an exercise? i have attached below the links of both the question and the solution i am working on.

https://www.dropbox.com/s/eulx25wkxmnd3f2/Programming%20principles%20exercise%202.jpg?dl=0

https://www.dropbox.com/s/q3osfqlfwbbe4dg/Source.cpp?dl=0

There are a couple of errors in the solution. I would be thankful if anyone could identify and correct those errors.

thank you.

[Tyler T]

First Error:

    //Line 12-33 and line 41-57 all have this problem
    for (int i = 0; i < 20; i++) //Looks pretty innocent, right?
    {
        if (num1[i] == 'I' || num1[i] == 'V' || num1[i] == 'X' || num1[i] == 'L' || num1[i] == 'C' || num1[i] == 'D' || num1[i] == 'M')
        {
            if (num1[i] == 'I')
                numf = numf + 1;
            if (num1[i] == 'V')
                numf = numf + 5;
            if (num1[i] == 'X')
                numf = numf + 10;
            if (num1[i] == 'L')
                numf = numf + 50;
            if (num1[i] == 'C')
                numf = numf + 100;
            if (num1[i] == 'D')
                numf = numf + 500;
            if (num1[i] == 'M')
                numf = numf + 1000;
        }
        else
            cout << "Its in invalid";
    }

The line of the for loop looks pretty innocent, right? But think about it. What if the user inputs a number, say "IIIX", that has less than 20 chars? Once you pass the "X", you encounter the null terminator. It doesn't match any of the valid chars, so it will output "invalid". AND once after the null terminator, the memory is unused by cin, because there are no more chars to read in, which gives you stack garbage. You can fix this by:
for(int i = 0;i < 20 && num1[i] != '\0';i ++)
Second Error:

if (oper == '+')
    {
        sum = numf + nums ;
        cout << sum;
    }
         
    if (oper == '-')
    {
        sum = numf - nums;
        cout << subtract; //You are outputting subtract, but you are storing the result in sum
    }
    if (oper == '*')
    {
        sum = numf * nums;
        cout << Multiply; //Here also
    }
    if (oper == '/')
    {
        sum = numf / nums;
        cout << Divide; //And here
        //
    }

Other than that I don't see any errors, only that at some places your code can be improved, like using the += operator and using switch instead of ifs. But where is your code that outputs the result as a roman numeral? Do you have problem figuring that out?

P.S. You should use std::string instead of c-strings whenever available. What if the user inputted a roman numeral that has more than 20 chars? It would probably cause a memory corruption or something like that and crash your program. But std::string would allocate more memory from the heap for you, so that you don't have to worry about anything.

[shaidul]

Yes i have problem figuring out the last line of the output since i cant think of the logic to execute it.

[Tyler T]

Since that roman numerals have symbols for numbers like 5 and 50, I don't really have a good solution to it, so I have to do it the hard way:

void toRomanNumeral(char *out, int num) {
    int currentIndex = 0; //The current index we are writing to of out
    //With some math we can figure out the number of Ms, Ds, Cs, Ls, Xes, Vs, and Is
    int num1000 = num / 1000;
    int num500 = num % 1000 / 500;
    int num100 = num % 1000 % 500 / 100;
    int num50 = num % 100 / 50;
    int num10 = num % 100 % 50 / 10;
    int num5 = num % 10 / 5;
    int num1 = num % 10 % 5;
    //loop though and write the chars to out
    for(int i = 0; i < num1000; currentIndex ++, i ++) {
        out[currentIndex] = 'M';
    }
    for(int i = 0; i < num500; currentIndex ++, i ++) {
        out[currentIndex] = 'D';
    }
    for(int i = 0; i < num100; currentIndex ++, i ++) {
        out[currentIndex] = 'C';
    }
    for(int i = 0; i < num50; currentIndex ++, i ++) {
        out[currentIndex] = 'L';
    }
    for(int i = 0; i < num10; currentIndex ++, i ++) {
        out[currentIndex] = 'X';
    }
    for(int i = 0; i < num5; currentIndex ++, i ++) {
        out[currentIndex] = 'V';
    }
    for(int i = 0; i < num1; currentIndex ++, i ++) {
        out[currentIndex] = 'I';
    }
    out[currentIndex] = '\0'; //null terminator
}

And use it like this:

char romanNumeral[20];
toRomanNumeral(romanNumeral, 1234);
cout << romanNumeral << endl;

Note that the function returns void, since we are outputting the data into a pointer passed in.

[shaidul]

cout << "The sum of "<<num1 <<" and "<<num2<< " is "<<sum<<endl;

I have used the above code to display the last line of the output,but cant think of how to display the answer in roman numerals and the number in brackets.

[E0p9LyTq]

Displaying Arabic numbers in Roman Numerals, and vice versa, requires a converter with a lot of if statements, loops or switch cases.