CODEWITHSUNDEEP
cif-statement
Jakub Tětek
Jakub Tětek

Reputation: 161

Long if..else statement

How do I reduce long if..else statement such as this one?

if( strcmp( alnumToc, "log") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_log( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "log2") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_log2( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "log10") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_log10( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "sqrt") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_sqrt( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "cbrt") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_cbrt( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "abs") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_abs( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "sin") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_sin( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "cos") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_cos( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "csc") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_csc( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "cot") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_cot( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "acos") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_acos( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "asin") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_asin( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "atan") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_atan( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "cosh") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_cosh( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        } else if( strcmp( alnumToc, "sinh") == 0){
            ARGNUMCHECK( in, 1);
            mpfr_sinh( num, stack[j-1], MPFR_RNDN);
            CPYRES( 1);
        }

It's not just ugly but also slow. I thought of using hashtable with function pointers. Is that good solution?
Note: there will be some mpfr functions, that take more arguments, so I can't create just one macro.

Upvotes: 4

Views: 454

Answers (4)

user1375096
user1375096

Reputation:

The performance issue came from that strcmp repeatedly scan the same string over and over again.

If your input data is clean, means no unexpected content in alnumToc, you can actually skip lots of characters comparison to gain performance. Try this:

switch(alnumToc[0]) {
  case 'a': // abs, acos, asin or atan, you don't have to check beyond alnumToc[1]
    switch(alnumToc[1]) {
      case 'b'
        // deal with "abs"
        break;
      case 'c'
        // deal with "acos"
        break;
      case 's'
        // deal with "asin"
        break;
      case 't'
        // deal with "atan"
        break;
    }
    break;

  case 'l': // log, log2, or log10, "og" is common, no need to check
    switch(alnumToc[3]) {
      case '\0' // terminator of c string
        // deal with "log"
        break;
      case '2'
        // deal with "log2"
        break;
      case '1'
        // deal with "log10"
        break;
    }
    break;
  // you can figure out the rest
}

The bottom line is to only scan alnumToc once for best performance. If you cannot guarantee alnumToc is clean, exhause all alnumToc[0], [1], [2], ....

Upvotes: 1

Lundin
Lundin

Reputation: 213892

That is very poor code and indeed it will be very slow. It must be rewritten from scratch.

The fastest possible way is to store all string literals in a sorted array, then use bsearch to search through it. Use an array of structs where the string is the search key. You'll have to write your own comparison function for your struct type.

If you find the string you were looking for, call its corresponding function through the function pointer:

typedef struct
{
  const char*  STRING;
  func_t       action;
} my_str_thing;

const my_str_thing TABLE [N] =
{
  { "ABC", do_this},
  { "DEF", do_that},
  ...
};

A hash table would work out too, but seems needlessly complex for this specific case.

Upvotes: 6

Jongware
Jongware

Reputation: 22457

  1. Store all of the names and functions in a structure:

    struct calcRoutine_t {
  const char *name;
  void (*function)(int, int, int);
} calc_routine[] = {
  { "log",  mpfr_log },
  { "log2", mpfr_log2 },
  { "log10",    mpfr_log10 },
  { "sqrt", mpfr_sqrt },
  { "cbrt", mpfr_cbrt },
  { "abs",  mpfr_abs },
  { "sin",  mpfr_sin },
  { "cos",  mpfr_cos },
  { "csc",  mpfr_csc },
  { "cot",  mpfr_cot },
  { "acos", mpfr_acos },
  { "asin", mpfr_asin },
  { "atan", mpfr_atan },
  { "cosh", mpfr_cosh },
  { "sinh", mpfr_sinh }
};

  2. Loop over the array and use strcmp to locate the correct function:

    for (i=0; i<sizeof(calc_routine)/sizeof(calc_routine[0]); i++)
{
    if (!strcmp ( calc_routine[i], alnumToc) )
    {
        ARGNUMCHECK( in, 1);
        calc_routine[i].function (num, stack[j-1], MPFR_RNDN);
        CPYRES( 1);
        break;
    }
}

    (You can add a flag for 'success' before the break, or test if i == sizeof(calc_routine) /sizeof(calc_routine[0]) at the bottom.)

This has the initial advantage you can add, shuffle, and remove any of the sub-routines at will.

As soon as you decided on a final set of names/functions, sort them once by name and then use bsearch instead of this loop. On success, bsearch will point to the correct structure member and you can call its associated function right away; on failure, bsearch will return NULL.

Add

As noted in a comment, some functions may need more arguments than 1. This number can be stored in the struct calcRoutine_t as well, to be tested in the loop.

Upvotes: 4

Jeyaram
Jeyaram

Reputation: 9474

1. Instead keep all strings in an array.

char *strarray[] = {"log", "log2", "blah Blah"};

2. Run a for loop like

for(i = 0; i < NO_OF_STRINGS; i++)
{
    // strcmp
}

3. Keep array of function pointers for mpfr_sqrt, mpfr_acos,..

It will reduce code size.

Upvotes: 0

Related Questions