-4

Just going to change my question for now - I could just use some guidance as to why I have three compiler errors in my program, not quite sure what I did wrong/am missing - I have added comments into the code just to state where they are. Thanks

   #include <iostream>
   #include <string>


using namespace std;

class romanType
{

public:
    void setRomanNum(string store);
    // this function will store the Roman numeral

    int convertNum(char rNum);
    // this function will convert the Roman numeral to a decimal

    void decimalPrint(int total);
    // this function will print the decimal number

    void romanPrint(char rNum);
    // this function will print the Roman numeral

    int getNum(char letter);
    // this function will get the number input

    romanType(int store);
    //Constructor with parameter

    romanType();

    char roman[7];
    string num;
    int length = 0;
    string dNum;
    int equals;

};

romanType::romanType(int store)
{
    dNum = 1;

}


void romanType::setRomanNum (string store)
{
    dNum = store;
}

void romanType::romanPrint(char rNum)
{
    cout << "The Roman numeral is: " << roman << endl;
}


void romanType::decimalPrint(int total)
{
    cout << "The Decimal number is: " << equals << endl;
}


int romanType::convertNum (char rNum)
{

    int letter;
    int totalNum = 0;


    for (int i = 0; i< dNum.length(); i++)
        // "loop will run at most once (loop increment never executed)"?

    {

        switch (roman[i])
        {
            case 'M':
                totalNum+= 1000;
                break;

            case 'D':
                totalNum += 500;
                break;

            case 'C':
                totalNum += 100;
                break;

            case 'L':
                totalNum += 50;
                break;

            case 'X':
                totalNum += 10;
                break;

            case 'V':
                totalNum += 5;
                break;

            case 'I':
                totalNum += 1;
                break;

        }
        totalNum = totalNum + letter;
        equals = totalNum;

        return equals;
    }
};
// "control may reach end of non-void function"


int main()
{
    romanType output;


    int rNumeral;
    char entry;


    cout << "Please enter a Roman numeral (Capitalized only): " << endl;
    cin >> rNumeral;

    cout << "Print Decimal or Roman Numeral? Type 1 for Decimal, 2 for Roman Numeral: " << endl;
    cin >> entry;

    if (entry == '1')
    {
        cout << "You chose to view the decimal conversion." << endl;
    output.decimalPrint(rNum);
    // How do I output the decimal conversion from the void romanType::decimalPrint(int total) function?
    }
    else if (entry == '2')
    {


        cout << "You chose to view the Roman numeral." << endl;
    output.romanPrint(rNumeral);
    }
    else
        cout << "Error: bad input" << endl;
    return 0;
        exit(1);


}
Croset
  • 37
  • 6
  • 2
    One question per question, please. stackoverflow.com is not a tutorial, or a debugging service. State a concrete, discrete, cogent question, instead of asking everyone to look for all the question randomly sprinkled in your code, like some kind of an easter egg hunt. – Sam Varshavchik Sep 20 '16 at 22:58
  • 1
    Fix the compiler errors 1st, learn how to read them. After you have running code step through with a debugger to catchup with the logical errors. You can't expect us to do that for you here. – πάντα ῥεῖ Sep 20 '16 at 22:58
  • Just fixed a few of the compiler errors, I was unsure how to fix them - still have a couple in there though. If someone could help me with the compiler errors that would be great. I put in a couple comments where I'm getting the errors and don't understand why. I can ask about the logic in a separate question - just thought that was not allowed. Sorry. – Croset Sep 20 '16 at 23:22
  • You don't need 3 return statements, you need a default case in the switch stamement. Look it up. – iksemyonov Sep 20 '16 at 23:26
  • Your main mistake is that the Roman system is non-positional. You can't just add up consequent Roman digits to get the correct result. See en.wikipedia.org/wiki/Roman_numerals and en.wikipedia.org/wiki/Subtractive_notation – iksemyonov Sep 20 '16 at 23:32
  • That was my bad - I was getting an error and was messing about and forgot to fix it - I've fixed that now. – Croset Sep 20 '16 at 23:34
  • @iksemyonov thank you for the link - that's my main problem, I realize I need to somehow subtract per each numeral that is looped through but am unsure how to do that, hence the logic issue :( – Croset Sep 20 '16 at 23:36
  • Yes unfortunately that's something you have to work on yourself and only come back here when you've identified and narrowed down a specific problem, not just 'it doesn't work as expected". That's how Stack Overflow works! – iksemyonov Sep 20 '16 at 23:38
  • There are only 2 errors right now and both are discussed in my answer. Please look closer. Also reformat the code; it is *unacceptable* to treat your code like that let alone present it to the public unformatted. – iksemyonov Sep 21 '16 at 00:29

2 Answers2

4

The mistake in the algorithm is that the Roman system is non-positional. See en.wikipedia.org/wiki/Roman_numerals and en.wikipedia.org/wiki/Subtractive_notation. You can't just add up consequent digits (i.e. letters) but you also have to identify and account for the cases when subtraction takes place.

why am I getting the error "no matching constructor for initialization of "roman type"?

Because the only provided constructor is not the default one, but takes one parameter of type int. Since such a constructor was provided, the default constructor wasn't generated by the compiler. Define romanType::romanType() or change the existing one to romanType::romanType(int i = 0) (add a default parameter). See Is there an implicit default constructor in C++? and why default constructor is not available by default in some case

expected expression?

Provide braces around the preceding else block. More than one statement -> braces required.

if (entry == '1') {
    cout << "You chose to view the decimal conversion." << endl;
    output.decimalPrint(rNum);
} else if (entry == '2') 
    cout << "You chose to view the Roman numeral." << endl;
    output.romanPrint(rNumeral);
}

"control may reach end of non-void function"

This is a warning only, but it will turn into an error if you include the -Werror flag that tells the compiler to treat all warnings as errors.

Ok I was wrong on this one. Actually the trick is that it is (in theory) possible that the romanType::convertNum(int) function follows a route where the for loop will never get executed and thus no return statement will be executed either. That's bad since the function is declared to return int hence there must be present an explicit return statement that (surprise) would return a value. Move the return out of the loop. This error is also closely related to the next one, discussed below.

"loop will run at most once (loop increment never executed)"?

This is because the return statement is placed incorrectly: inside the for loop not outside of it, and inside the function body. Hence the loop runs once and the function returns. Move the return statement. Credit to @ATN_LR_boom for noticing this!

Also, please get in the habit of formatting code properly. It will save you a lot of headache down the way.

Other than that, I'd use a std::map for the conversion function, it's shorter and more clear to the reader compared to the switch statement. Something along the lines of

int romanType::convertNum(int rNum) {
    const static std::map<char, int> conversion = {
        {'M', 1000},
        {'D', 500},
        // more mappings
        {'I', 1}
    };
    if ((auto it = conversion.find(rNum)) != conversion.end())
        return it->second;
    else
        return -1;
}
Community
  • 1
  • 1
iksemyonov
  • 4,106
  • 1
  • 22
  • 42
1

Your logic for the switch is wrong, try something like this:

int totalNum = 0;
for (int i = 0; i< dNum.length(); i++)
// the loop ran once because you were returning values when catching a letter
{
    switch (roman[i])
    {
        case 'M': // roman[i] is a char, the cases should try to catch chars
            totalNum += 1000; // increment your global number
            break;

        case 'D':
             totalNum += 500;
            break;
        ...
    }
return totalNum;