Question

I have researched my issue all over StackOverflow and multi-google links, and I am still confused. I figured the best thing for me is ask...

Im creating a simple command line calculator. Here is my code so far:

const std::string Calculator::SIN("sin");  
const std::string Calculator::COS("cos");  
const std::string Calculator::TAN("tan");  
const std::string Calculator::LOG( "log" );  
const std::string Calculator::LOG10( "log10" );

void Calculator::set_command( std::string cmd ) {

    for(unsigned i = 0; i < cmd.length(); i++)
    {
    cmd[i] = tolower(cmd[i]);
    }

    command = cmd;
}

bool Calculator::is_legal_command() const {

    switch(command)
    {
    case TAN:
    case SIN:
    case COS:
    case LOG:
    case LOG10:
        return true;
        break;
    default:
        return false;
        break;
    }

}

the error i get is:

Calculator.cpp: In member function 'bool Calculator::is_trig_command() const':  
Calculator.cpp: error: switch quantity not an integer  
Calculator.cpp: error: 'Calculator::TAN' cannot appear in a constant-expression  
Calculator.cpp: error: 'Calculator::SIN' cannot appear in a constant-expression  
Calculator.cpp: error: 'Calculator::COS' cannot appear in a constant-expression  

The mighty internet, it says strings are allowed to be used in switch statements.

Thanks everyone, I appreciate your help.

Was it helpful?

Solution

In switch, the expression must be of "an integral type or of a class type for which there is an unambiguous conversion to integral type" (quoting VS2008 docs).

A string class doesn't have "unambiguous conversion to integral type", like a char does.

As a work-around:

  1. Create a map<string, int> and switch on the value of the map: switch(command_map[command]) `

  2. Do a set of if/else instead of switch. Much more annoying and hard to read, so I'd recommend the map route.

As an aside, an even better solution for really complicated logic like that is to improve the mapping solution to get rid of switch completely and instead go with a function lookup: std::map<std::string, functionPointerType>. It may not be needed for your specific case, but is MUCH faster for complicated very long look-up logic.

OTHER TIPS

As others and the compiler commented, strings are not allowed with switch. I would just use if

bool Calculator::is_legal_command() const {
    if(command == TAN) return true;
    if(command == SIN) return true;
    if(command == COS) return true;
    if(command == LOG) return true;
    if(command == LOG10) return true;
    return false;
}

I don't think that's any more complicated, and it's about as fast as it could get. You could also use my switch macro, making it look like

bool Calculator::is_legal_command() const {
    sswitch(command)
    {
    scase (TAN):
    scase (SIN):
    scase (COS):
    scase (LOG):
    scase (LOG10):
        return true;

    sdefault():
        return false;
    }
}

(having break after a return is dead code, and so should be avoided).

Strings cannot be used in switch statements in C++. You'll need to turn this into if/else if, like this:

if (command == "tan")
{
    // ...
}
else if (command == "cos")
{
    // ...
}
// ...

Rather than a switch.

I would use a command pattern. Then use a std::map to map the function name to the command object.

Something like this:

#include <math.h>
#include <map>
#include <string>
#include <iostream>

class Function
{
    public:
        // Easy public API that just uses the normal function call symantics
        double   operator()(double value)   { return this->doWork(value);}
        virtual ~Function()     {}
    private:
        // Virtual function where the work is done.
        virtual double doWork(double value) = 0;
};

// A sin/cos function
class Sin: public Function      { virtual double doWork(double value)     { return sin(value); } };
class Cos: public Function      { virtual double doWork(double value)     { return cos(value); } };

// A class that holds all the functions.
// A function name is mapped to a function object.
class FuncMap
{
    public:
        FuncMap()
        {
            // Constructor sets up the map
            functions["sin"]    = &sinFunc;
            functions["cos"]    = &cosFunc;
        }
        Function*   getFunction(std::string command) const
        { 
            // Default result not found.
            Function* result    = NULL;
            std::map<std::string, Function*>::const_iterator    find;

            // Look in the map to see if we find the value.
            // If it exists then find will not point at end()
            if ((find = functions.find(command)) != functions.end())
            {
                // Get the pointer to the function
                result  = find->second;
            }
            return result;
        }
    private:
    Sin     sinFunc;
    Cos     cosFunc;

    std::map<std::string, Function*>    functions;
};

// Declaring it globally for ease of use.
FuncMap     functions;


int main()
{
    // SImple example of usage.
    Function*   func    = functions.getFunction("sin");
    if (func == NULL)
    {
        std::cout << "No Function sin()\n";
        exit(1);
    }
    std::cout << "Result: " << (*func)(12.34) << "\n";
}

Not sure which mighty Internet you've been reading, but C++ doesn't allow strings in switch statements. (C# does, though.)

You need to convert your switch statement to a chain of if-else if-else statements that test equality.

The compiler error tells you everything you need to know. Only integral types may be compared in switch statements.

I'm not sure which "mighty internet" told you otherwise, but it was mighty wrong.

Strings cannot be used as constants in switch statements in c++. You can either use a map, a series of if's or you can move from representing your commands as strings to an enum. Parse from string to enum once, then use a switch like you do now. Note that your string parsing may require the same mechanism (map/if's), but depending on your use case using one approach over the other may improve readability. I'm not going to say anything on which approach is more readable.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top