5

I have a question about whether to use 'case' or 'ifs' in a function that gets called quite a lot. Here's the following as it is now, in 'ifs'; the code is self-explanatory:

int identifyMsg(char* textbuff) {
if (!strcmp(textbuff,"text")) {
    return 1;
}
if (!strcmp(textbuff,"name")) {
    return 2;
}
if (!strcmp(textbuff,"list")) {
    return 3;
}
if (!strcmp(textbuff,"remv")) {
    return 4;
}
if (!strcmp(textbuff,"ipad")) {
    return 5;
}
if (!strcmp(textbuff,"iprm")) {
    return 6;
}
return 0;
}

My question is: Would a switch perform better? I know if using 'ifs', I can place the most likely options at the top.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
KaiserJohaan
  • 9,028
  • 20
  • 112
  • 199
  • See this : http://stackoverflow.com/questions/97987/switch-vs-if-else – Nawaz Jan 30 '11 at 12:53
  • 1
    You ought to concentrate on making your code correct before you worry about micro-optimisation. And once it's correct, you should measure before you optimise. And then when you think you need to optimise you should think long and hard about the future maintainability of the optimised code. In short, there is no point in having optimised code that is incorrect. – David Heffernan Jan 30 '11 at 12:57

7 Answers7

11

You can't use switch statements for strings because they are pointers and are not evaluted at compile time. You are stuck with using a bunch of if statements.

However for the sake of performance, I believe switches perform better when there are more conditions to check but the difference will be so minute it wouldn't matter.

I've never tested this before though, but I've read about this kind of switch optimization:

switch (value) {
  case frequent_value1:
  case frequent_value2:
  case frequent_value3:
    break;

default:
  switch (value) {
     case infrequent_value1:
     case infrequent_value2:
     case infrequent_value3:
        break;
     }
}
Marlon
  • 19,924
  • 12
  • 70
  • 101
  • Indeed. But apart from that: the question "which performs better" would be moot even if it was possible. –  Jan 30 '11 at 12:51
  • Another good topic on SO : http://stackoverflow.com/questions/97987/switch-vs-if-else – Nawaz Jan 30 '11 at 12:52
  • 3
    Those nested switches shouldn't have any positive performance impact. If one of the frequent cases occurs, no conditions will be checked for any of the other cases anyway. Same goes for regular if-elseif. – Thorarin Jan 30 '11 at 12:58
  • 1
    There are various tricks involving hashing and code generators that enable switches for strings, though. Not worth the trouble, in my opinion, unless profiling shows that the performance of that particular part is a real problem. – Sergei Tachenov Jan 30 '11 at 13:10
5

You could use gperf to generate perfect hashes of the "verbs" you want to see. Then you could use a switch statement.

Or, you could do something like this:

switch (textbuff[0])
{
    case 'i':
    {
        switch (textbuff[1])
        {
            case 'p':
            {
                 switch (textbuff[2])
                 {
                     case 'a': /* something. */ break;
                     case 'r': /* something else. */ break;
                 }
            }
        }
    }

(You get the idea).

As yet another option (if all of your commands are 4 characters), turn them into a single 32-bit number and then switch on that:

int32_t mashed =
    textbuff[0] << 24 | 
    textbuff[1] << 16 |
    textbuff[2] << 8 |
    textbuff[3];

switch (mashed) { /* ... */ }

To be honest, though, unless the list of options is particularly large, or this function is being called an obscene number of times, it won't be worth it.

Remember: measure first; optimise later (only if necessary).

Roger Lipscombe
  • 89,048
  • 55
  • 235
  • 380
4

You could put all the values into a std::map.

class MessageMap
{ 
    std::map<std::string,int>    data;
    public:
        MessageMap()
        {
             data["text"]   = 1;
             data["name"]   = 2;
             data["list"]   = 3;
             data["remv"]   = 4;
             data["ipad"]   = 5;
             data["iprm"]   = 6;
        }
        int getMessageId(std::string const& index) cosnt
        {
            std::map<std::string,int>::const_iterator f;
            if ((f = data.find(index)) != data.end())
            {
                return f->second;
            }
            return 0;
        }
};
int identifyMsg(char* textbuff)
{
    static MessageMap   mssageMap;
    return messageMap.getMessageId(textbuff);
}
Martin York
  • 257,169
  • 86
  • 333
  • 562
2

You could very well use "enum" for those strings. then use switch case statements.

Manish Mulani
  • 7,125
  • 9
  • 43
  • 45
  • 1
    +1 by mistake: :-) Actually, that's what he is doing - converting a id string in a number id. If the id comes from data (e.g. XML), it hardly can be a number, but never an enum. – Valentin H Jan 30 '11 at 13:45
2

Another alternative that I have come across recently which might or might not fit your liking is:

int identifyMsg(const char* textbuff) {
    static const struct { const char* str; int id; } pairs[] = {
        { "text", 1 },
        { "name", 2 },
        { "list", 3 },
        { "remv", 4 },
        { "ipad", 5 },
        { "iprm", 6 },
    };
    for (int i = 0; i < sizeof(pairs)/sizeof(pairs[0]); ++i) {
        if (!strcmp(textbuff, pairs[i].str))
            return pairs[i].id;
    }
    return 0;
}
lumor
  • 36
  • 4
  • Why not simply use an STL associative array, then? – riviera Jan 30 '11 at 15:38
  • If you make pairs a `static` member of the function it will not be potentially re-bult each time the function is entered. – Martin York Jan 30 '11 at 17:59
  • For so few elements, an STL associative array would probably be less performant. I would start leaning towards an STL map<> in this case after about 10-20 elements if I was concerned about performance. This is mostly gut feeling though. Profiling should be used to determine where the two meet in performance if necessary. – lumor Jan 31 '11 at 21:51
2

There were two questions, as far as I understood. Optimization and if/switch.

First of all, code-optimization is a costly process. Optimize only those parts of code, which are bottle-necks by evident. I doubt it in this case. Looks, like you are dispatching a textual-id for making a decision what to do with a message. Where does the message come from? IPC, XML, File? What are you going to do with this message? How performant is the message-content processing-code? There should be places in code, which take more ressources than string comparison.

Have you tried out some performance analyzers like Purify, gperf, cachegrind?

Concerning if/switch: switch works only with integer-types. (char, short, int, long, enum)

Valentin H
  • 7,240
  • 12
  • 61
  • 111
2

Assuming it actually matters:

Because strcmp is slow, but an integer compare is fast: if all your commands are 4 characters long - which happens to fit in a 32bit integer - you can cast each string to a 32bit number, and then switch based on that.

Otherwise, there are two basic ways to quickly compare a string against a lot of candidate strings:

  • Store the candidates in a hash table.

or

  • Sort the candidates alphabetically sorted in an array. You can then perform a binary search on the array by using the result of strcmp to either find a match, or exclude half of the remaining candidates.

As a side note - Compilers, such as MSVC and GCC, have implemented an optimization on switches that tests the switch conditions using a binary search. So a switch statement with, say, 256 elements, will be optimized down to, at most, 8 compare operations.

Chris Becke
  • 34,244
  • 12
  • 79
  • 148