views:

226

answers:

8

I have this huge switch case with nested switch case statements in it that I was wondering if anyone had any ideas on how to clean up?

switch (datatype) {
    case STR:
    {
         switch(operation)
         {
              case EQUALS:
              {
                     /* perform str compare */
              }
              case NOTEQUALS:
              {
              }
              case LT:
              {
              }
              case GT:
              {
              }
              case GTE:
              {
              }
              case LTE:
              {
              }
              default:
                 break;
         }
         break;
    }
    case VER:
    {
         switch(operation)
         {
              case EQUALS:
              {
                     /* perform version compare */
              }
              case NOTEQUALS:
              {
              }
              case LT:
              {
              }
              case GT:
              {
              }
              case GTE:
              {
              }
              case LTE:
              {
              }
              default:
                 break;
         }
         break;
    }
    case INT:
    {
         /* same */
    }
    case FLOAT:
    {
         /* same */
    }
    /* ... more types ... */
    /* ... .......... ... */
    default:
        break;
}
+11  A: 

If the values for the operation are contiguous, you could make a table of function pointers. In fact you could make a 2D table of function pointers with a separate function to handle each operation/type combination. e.g

// do some range checking on input params
// invoke the handler
handlers[datatype][operation]();
Stephen Doyle
you might want to do some verification that `datatype` and `operation` are in the correct range, otherwise Bad Things will happen.
rmeador
Actually modern compilers will convert large switches to jumptables automatically (and do the range checking of course)
drhirsch
@drhirsch: this isn't about performance, it's about making clean code.
rmeador
This doesn't seem cleaner to me. It will be more opaque and more difficult to maintain. I don't think there is anything inherently wrong with the long switch statements.
Kinopiko
@Kinopiko: I have to disagree. Tables of function pointers are one the best ways to make complex C code like this easier to understand and maintain.
bcat
+2  A: 

You could try the command pattern.

AraK
A: 

You could use a big array of function pointers and then call the relevant function based upon indexing to the correct function pointer in the array.

Goz
+5  A: 

Create some tables (arrays) with pointers to functions in it. You can then look up func[STR][EQUALS] to make the appropriate call. The call would end up looking like this...

Func[datatype][operation]();
rikh
+2  A: 

The NOTEQUALS case can always be written in terms of the EQUALS case; similarly GTE in terms of LT and LTE in terms of GE. So make the outer switch in terms of the operation, and only three of those six cases will need to switch on the datatype.

Dave Hinton
Not always if you consider partial orderings.
liori
A: 

have you considered creatively using a series of function pointers and storing them in a struct?

do it right and you can mimic objects and do something this like:

bool someArbitraryFunction (dataType aDatatype, operations anOperation)
{
 someUnknownStruct.datatype = aDatatype;
 someUnknownStruct.operation = anOperation;
 return someUnknownStruct->doMath(1,2);
}

and then you can put all the needed math functions, enums, and structs in a header file somewhere.

cleans up the "meat" of the code, and makes the math portable (just import it wherever you need it).

pxl
A: 

Try this post

link text

It's talks about command pattern in C .

Night Walker
A: 

Assuming your cases can all return a simple boolean value, all six logic cases can be rewritten in terms of LT and EQUALS, like follows:

switch(operation) {
  case EQUALS:
    return isEquals();
  case NOTEQUALS:
    return !isEquals();
  case LT:
    return isLT();
  case GT:
    return !isLT() && !isEquals();
  case GTE:
    return !isLT();
  case LTE:
    reutrn isLT() || isEquals();
  default:
    break;
}

This would only require you to write the logic for isLT() and isEquals(), which would do the switching on datatype when necessary. This eliminates a significant amount of unnecessary code duplication, yet doesn't sacrifice a lot of legibility.

This can be combined with function pointers as Stephen Doyle and rikh already suggested, which would eliminate the switch() statement entirely.

goldPseudo