Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is there a better way to rewrite this ugly switch and if statement combination?

Essentially I have a system of gamma detectors that are segmented into 4 crystals each, in the case when only 2 of the of crystals register a hit we can determine if the pair was perpendicular or parallel to the plane of the reaction generating the gamma-ray. In the process of writing up the logic for this I wound up writing a huge and ugly combination of switch statements which in each detector checks for the combinations of crystal numbers (which are unique across the whole array of detectors and their crystals). Here is the code, including the function in question.

//The Parallel and Perpendicular designations are used in addition to the Double
//designation for the 90 degree detectors if we get a diagonal scatter in those detectors
//then we use the Double designation
enum ScatterType{Single, Double, Triple, Quadruple, Parallel, Perpendicular};

ScatterType EventBuffer::checkDoubleGamma(int det)
{
    int num1=evList[crysList[0]].crystalNum;
    int num2=evList[crysList[1]].crystalNum;

    switch(det)
    {
    case 10: //first of the 90 degree detectors
        if( (num1==40 && num2==41) || //combo 1
            (num1==41 && num2==40) || //combo 1 reverse
            (num1==42 && num2==43) || //combo 2
            (num1==43 && num2==42)   )//combo 2 reverse
        { return Parallel; }
        else if( (num1==40 && num2==42) || //combo 1
                 (num1==42 && num2==40) || //combo 1 reverse
                 (num1==41 && num2==43) || //combo 2
                 (num1==43 && num2==41)   )//combo 2 reverse
        { return Perpendicular; }
        else
        { return Double;}
        break;
    case 11: //second of the 90 degree detectors
        if( (num1==44 && num2==45) || //combo 1
            (num1==45 && num2==44) || //combo 1 reverse
            (num1==46 && num2==47) || //combo 2
            (num1==47 && num2==46)   )//combo 2 reverse
        { return Parallel; }
        else if( (num1==44 && num2==47) || //combo 1
                 (num1==47 && num2==44) || //combo 1 reverse
                 (num1==45 && num2==46) || //combo 2
                 (num1==46 && num2==45)   )//combo 2 reverse
        { return Perpendicular; }
        else
        { return Double;}
        break;
    case 13: //third of the 90 degree detectors
        if( (num1==52 && num2==53) || //combo 1
            (num1==53 && num2==52) || //combo 1 reverse
            (num1==54 && num2==55) || //combo 2
            (num1==55 && num2==54)   )//combo 2 reverse
        { return Parallel; }
        else if( (num1==52 && num2==55) || //combo 1
                 (num1==55 && num2==52) || //combo 1 reverse
                 (num1==53 && num2==54) || //combo 2
                 (num1==54 && num2==53)   )//combo 2 reverse
        { return Perpendicular; }
        else
        { return Double;}
        break;
    case 14: //fourth of the 90 degree detectors
        if( (num1==56 && num2==57) || //combo 1
            (num1==57 && num2==56) || //combo 1 reverse
            (num1==58 && num2==59) || //combo 2
            (num1==59 && num2==58)   )//combo 2 reverse
        { return Parallel; }
        else if( (num1==56 && num2==59) || //combo 1
                 (num1==59 && num2==56) || //combo 1 reverse
                 (num1==57 && num2==58) || //combo 2
                 (num1==58 && num2==57)   )//combo 2 reverse
        { return Perpendicular; }
        else
        { return Double;}
        break;
    default:
        throw string("made it to default case in checkDoubleGamma switch statement, something is wrong");
        break;
    }
}

I am aware that, because the crystal numbers are global rather than per detector, I could do away with the switch statement and have an enormous set of conditionals linked by or statements, essentially reducing things to 3 control paths, one returning Parallel, one returning Perpendicular, and one returning Double, instead of the 12 control paths with 4 of each that I have. I initially wrote it as it is not thinking and in all honesty, thinking about it, this method reduces the average case number of boolean statements to grind through.

I just worked out how to make it more efficient by switching on the evList[crysList[0]].crystalNum I can reduce the evaluations quite a bit, yielding this:

ScatterType EventBuffer::checkDoubleGamma()
{
    int crysNum = crysList[1].crystalNum;
    switch(evList[crysList[0]].crystalNum)
    {
    case 40:
        if (crysNum == 41) {return Parallel;}
        else if (crysNum == 42) {return Perpendicular;}
        else {return Double;}
        break;
    case 41:
        if (crysNum == 40) {return Parallel;}
        else if (crysNum == 43) {return Perpendicular;}
        else {return Double;}
        break;
    case 42:
        if (crysNum == 43) {return Parallel;}
        else if (crysNum == 40) {return Perpendicular;}
        else {return Double;}
        break;
    case 43:
        if (crysNum == 42) {return Parallel;}
        else if (crysNum == 41) {return Perpendicular;}
        else {return Double;}
        break;
    case 44:
        if (crysNum == 45) {return Parallel;}
        else if (crysNum == 47) {return Perpendicular;}
        else {return Double;}
        break;
    case 45:
        if (crysNum == 44) {return Parallel;}
        else if (crysNum == 46) {return Perpendicular;}
        else {return Double;}
        break;
    case 46:
        if (crysNum == 47) {return Parallel;}
        else if (crysNum == 45) {return Perpendicular;}
        else {return Double;}
        break;
    case 47:
        if (crysNum == 46) {return Parallel;}
        else if (crysNum == 44) {return Perpendicular;}
        else {return Double;}
        break;
    case 52:
        if (crysNum == 53) {return Parallel;}
        else if (crysNum == 55) {return Perpendicular;}
        else {return Double;}
        break;
    case 53:
        if (crysNum == 52) {return Parallel;}
        else if (crysNum == 54) {return Perpendicular;}
        else {return Double;}
        break;
    case 54:
        if (crysNum == 55) {return Parallel;}
        else if (crysNum == 53) {return Perpendicular;}
        else {return Double;}
        break;
    case 55:
        if (crysNum == 54) {return Parallel;}
        else if (crysNum == 52) {return Perpendicular;}
        else {return Double;}
        break;
    case 56:
        if (crysNum == 57) {return Parallel;}
        else if (crysNum == 59) {return Perpendicular;}
        else {return Double;}
        break;
    case 57:
        if (crysNum == 56) {return Parallel;}
        else if (crysNum == 58) {return Perpendicular;}
        else {return Double;}
        break;
    case 58:
        if (crysNum == 59) {return Parallel;}
        else if (crysNum == 57) {return Perpendicular;}
        else {return Double;}
        break;
    case 59:
        if (crysNum == 58) {return Parallel;}
        else if (crysNum == 56) {return Perpendicular;}
        else {return Double;}
        break;
    default:
        throw string("made it to default case in checkDoubleGamma switch statement, something is wrong");
        break;
    }
}

The question still remains though, is there any trick to make this shorter? more efficient? more readable?

Thanks in advance!

like image 954
James Matta Avatar asked Feb 06 '14 05:02

James Matta


People also ask

Which is better to use if statement or switch statement?

A switch statement is significantly faster than an if-else ladder if there are many nested if-else's involved. This is due to the creation of a jump table for switch during compilation. As a result, instead of checking which case is satisfied throughout execution, it just decides which case must be completed.

How is the if-else if combination more general than a switch statement?

Answer. Answer: A switch statement is usually more efficient than a set of nested ifs. ... Check the Testing Expression: An if-then-else statement can test expressions based on ranges of values or conditions, whereas a switch statement tests expressions based only on a single integer, enumerated value, or String object ...

Which is faster if statement or switch?

As it turns out, the switch statement is faster in most cases when compared to if-else , but significantly faster only when the number of conditions is large. The primary difference in performance between the two is that the incremental cost of an additional condition is larger for if-else than it is for switch .

Can you put a switch in an if statement?

Note that you can also use a switch inside a switch, just like nested if statements. Save this answer. Show activity on this post. If you have only 2 or 3 option then you can use if else otherwise use nested switch.


2 Answers

I think you can move nearly everything into a simple table and get away with a single table lookup. I haven't studied your conditions in detail, but it looks like something like this will do the job just fine:

// fill the following table in advance using your existing function, or hard-code the 
// values if you know they will never change:
ScatterType hitTable[60][60];


ScatterType EventBuffer::checkDoubleHit(int det)
{
    // read the crystal Nums once:
    unsigned a = evList[cryList[0]].crystalNum;
    unsigned b = evList[cryList[1]].crystalNum;

    switch(det)
    {
    case 10:
    case 11:
    case 13: 
    case 14:
      // better safe than sorry:
      assert (a < 60);
      assert (b < 60);
      return hitTable[a][b];
    break;

    default:
        throw string("made it to default case in checkDoubleHit switch statement, something is wrong");
        break;
    }
}
like image 126
Nils Pipenbrinck Avatar answered Sep 21 '22 06:09

Nils Pipenbrinck


One solution to make it shorter is to sort the crystal values before comparing/switching

int nummin=evList[crysList[0]].crystalNum;
int nummax=evList[crysList[1]].crystalNum;

if (nummin > nummax) 
{
    tmp = nummin;
    nummin = nummax;
    nummax = tmp;
}
// or like Jarod42 said: std::minmax(numin, numax);

if ((nummin == 40 && nummax == 41) ||  // no need to compare the reverse
    (nummin == 42 && nummax == 43))    // and reduce haft of the comparison
{ ... }
like image 25
phuclv Avatar answered Sep 19 '22 06:09

phuclv