0

I was helping a friend with one of his C++ assignments and we found the following code snippet would throw exceptions in MSVC, but when compiling with G++, the exact same code would work fine. The exceptions were return because this function called getValue() wasn't returning anything.

string getValue(int value) {

    ostringstream convert;

    string rtnValue;

    switch (value) {
    case 11:
    {
        rtnValue = "J";
        break;
    }
    case 12:
    {
        rtnValue = "Q";
        break;
    }
    case 13:
    {
        rtnValue = "K";
        break;
    }
    case 14:
    {
        rtnValue = "A";
        break;
    }
    default:
    {
        //
        // if the value is a a number, we assume it is 2..10
        //
        convert << value;            // use a stream to convert the number
        rtnValue = convert.str();    // into a string
        if (value < 2 || value > 10)
        {
            rtnValue = "ERROR" + rtnValue + "ERROR";
        }
    }
    return rtnValue;
    }
}

This program turns integers into strings. For the numbers 11-14 it uses switch statement (I know this isn't the best implementation but it's an introductory class).

We found that this could easily be solved by adding another return statement at the end.

string getValue(int value) {

    ostringstream convert;

    string rtnValue;

    switch (value) {
        case 11:
        {
            rtnValue = "J";
            break;
        }
        case 12:
        {
            rtnValue = "Q";
            break;
        }
        case 13:
        {
            rtnValue = "K";
            break;
        }
        case 14:
        {
            rtnValue = "A";
            break;
        }
        default:
        {
            //
            // if the value is a a number, we assume it is 2..10
            //
            convert << value;            // use a stream to convert the number
            rtnValue = convert.str();    // into a string
            if (value < 2 || value > 10)
            {
                rtnValue = "ERROR" + rtnValue + "ERROR";
            }
        }
        return rtnValue;
    }
    return rtnValue;
}

And this now fixes it for MSVC (and I assume G++ if I checked).

Why did that fix work? Does MSVC and G++ treat parentheses differently with respect to switch statements?

Cameron
  • 2,805
  • 3
  • 31
  • 45

3 Answers3

3

In the first example, the return rtnValue is in the wrong place, and will only ever work when the default case is hit.

In the second example, you have added the return rtnValue in the correct place (and the other can be safely removed).

As to why it worked on GCC and not on MSVC, I don't know, without the return being in the correct place, it's not valid C++ (not all paths have a return value), so you should have got a compilation error on any C++ compiler.

I would suggest the problem is actually the way the braces {} are being used, and your friend thought that the closing brace of the default case, actually closed the switch statement, but it doesn't.

Also, there is no need to have braces on any of the case statements. Braces CAN be used in this way to introduce scoping (for example, temporary variables for a particular case), but in your example, just leads to confusion.

Neil
  • 11,059
  • 3
  • 31
  • 56
0

Return value

Your return statement in the first sample applies to the default case only since the execution of the switch block ends with a break statement in every other case.

In a non-default case, you leave the return value of your function uninitialized. MSVC does warn about that while debugging (see https://learn.microsoft.com/en-us/visualstudio/debugger/how-to-use-native-run-time-checks for details) but GCC does not. This problem might be detected during compile time but you cannot rely on that.

The return statement added to the second sample is correct. You can remove the original one which becomes superfluous.

Braces

Notice that the braces inside the switch block are not necessary and introduce confusion here. They would be only useful if you created a local variable just to be used in a single case. Anyway, the braces should be indented more than the braces of the switch block. This part

}
return rtnValue;
}

demonstrates the misleading indentation clearly. The indentation used in the second example is one of the good solutions to this problem.

Melebius
  • 6,183
  • 4
  • 39
  • 52
0

this is the problem

 default:
{

    convert << value;            // use a stream to convert the number
    rtnValue = convert.str();    // into a string
    if (value < 2 || value > 10)
    {
        rtnValue = "ERROR" + rtnValue + "ERROR";
    }
}
return rtnValue;
}

your return statement is in the wrong block, i.e , switch block.

what happens is that, when a case is satisfied it breaks out of the switch that is why it didn't return anything (because it is now out of switch statement).

In order to fix it you have to move your return statement to out of the switch statement to the end of the function.

This correction will we equivalent to the second code that you have provided.

But even in the second code remove the inner return statement.

aditya rawat
  • 154
  • 10