Bool value seemingly ignored in if statement

[romey]

When I run my code, it is processing past the if statements whether or not they are being evaluated correctly.

If you run it, you will notice that whatever input you provide, the program will return "test", and "0result".

"Test" output only exists to check if the isArabic if statement is evaluating.
0 is the expected output if no roman numeral was entered. (int results is initialized with the value 0)

Any insight is greatly appreciated.

/**
 * Computer Programming 2 (COMP2711, COMP8801)
 * Practical 2: Roman Numbers
 */

#include <iostream>
#include <string>
#include <sstream>
#include <cctype>


using namespace std;

int main(int argc, char *argv[]) {
    const string units[] = {"ZERO", "I", "II", "III", "IV", "V", "VI", "VII", "VIII", "IX"};
    const string tens[] = {"ZERO", "X", "XX", "XXX", "XL", "L", "LX", "LXX", "LXXX", "XC"};
    const string hundreds[] = {"ZERO", "C", "CC", "CCC", "CD", "D", "DC", "DCC", "DCCC", "CM"};
    const string thousands[] = {"ZERO", "M", "MM", "MMM"};
    const string arabic[] = {"0","1", "2", "3", "4", "5", "6", "7", "8", "9"};

    string input;

    while (cin >> input) {

        bool isArabic = false;

        for (int k = 0; k < (sizeof(arabic) / sizeof(arabic[0])) - 1; ++k) { // Find if first digit is an arabic numeral
            if (input.find(arabic[k]) == 0) {
                isArabic = true;
                break;
            }
        }

        if (isArabic);
        {
            string number;
            for (int i = 0; i < (sizeof(arabic) / sizeof(arabic[0]) - 1); ++i) { //remove all prefixed arabic numerals and put into number variable
                if (input.find(arabic[i]) == 0) {
                    cout << "arabic[i]" << arabic[i] << '\n';
                    number = number + arabic[i];
                    input.erase(0, (1));
                    i = 0;
                }
            }
            //input = "";
            cout << number << "test";
        }

        for (int i = 0; i < input.length(); ++i) {      //Make input uppercase
            input.at(i) = toupper(input.at(i));
        }

        if (
                (!isArabic && input.find("I") == 0) ||
                (!isArabic && input.find("V") == 0) ||
                (!isArabic && input.find("X") == 0) ||
                (!isArabic && input.find("L") == 0) ||
                (!isArabic && input.find("C") == 0) ||
                (!isArabic && input.find("D") == 0) ||
                (!isArabic && input.find("M") == 0));
        {
            int result = 0;
            for (int j = (sizeof(thousands) / sizeof(thousands[0])) - 1; j > 0; --j) {
                if ((input.find(thousands[j]) ==
                     0)) { //if value of thousands index j is found at position 0 of input
                    result += (j * 1000);
                    input.erase(0, (thousands[j].length())); //Removes roman numerals from string
                    break;
                }
            }

            for (int j = (sizeof(hundreds) / sizeof(hundreds[0])) - 1; j > 0; --j) {
                if ((input.find(hundreds[j]) == 0)) {
                    result += (j * 100);
                    input.erase(0, (hundreds[j].length()));
                    break;
                }
            }

            for (int j = (sizeof(tens) / sizeof(tens[0])) - 1; j > 0; --j) {
                if ((input.find(tens[j]) == 0)) {
                    result += (j * 10);
                    input.erase(0, (tens[j].length()));
                    break;
                }
            }

            for (int j = (sizeof(units) / sizeof(units[0])) - 1; j > 0; --j) {
                if ((input.find(units[j]) == 0)) {
                    result += j;
                    input.erase(0, (units[j].length()));
                    break;
                }
            }

            cout << result << "result"<<'\n';

        }

    }
}

[romey]

I see it now. Haha, I put ; after the if statement

[Handy Andy]

Hello romey,

I have a question. In the first for loop, line 28, do you only need to check the first character of the string or every character of the string?

Some tips.

For the {}s it is best to pick one style and be consistent in its use. Mixing styles makes it harder to read and that is not what you want.

I prefer the "Allman" style see https://en.wikipedia.org/wiki/Indentation_style#Brace_placement_in_compound_statements it just seams the easiest to read.

The for loop, line 50, would work better as:
for (size_t i = 0; i < input.length(); ++i) because the ".length" and ".size" functions both return a type of "size_t" which is an "unsigned integral type". This could be an "int" or a "long" which depends on the header files and compiler setup. So comparing a signed int "i" to an unsigned something should produce a compiler warning.

const string arabic[] = {"0","1", "2", "3", "4", "5", "6", "7", "8", "9"}; may work for you, but const std::string arabic{ "0123456789" } will work the same.

The use of "input.at(i)" does have its use, but in a for loop that limits the iterations to be less than the strings size the ".at()" is more overhead than you need. "input[i]" would be the better choice.

Just some thoughts for now

Andy