【问题标题】:Is there a better way to rewrite this ugly switch and if statement combination?有没有更好的方法来重写这个丑陋的开关和 if 语句组合?
【发布时间】:2014-02-06 05:59:53
【问题描述】:

基本上我有一个伽马探测器系统,每个探测器被分割成 4 个晶体,在只有 2 个晶体记录撞击的情况下,我们可以确定这对是垂直还是平行于产生反应的平面伽马射线。在为此编写逻辑的过程中,我最终编写了一个巨大而丑陋的 switch 语句组合,在每个探测器中检查晶体编号的组合(在整个探测器及其晶体阵列中是唯一的)。 这是代码,包括有问题的函数。

//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;
    }
}

我知道,由于水晶数字是全局的,而不是每个检测器,我可以取消 switch 语句,并通过 or 语句链接大量条件,基本上将事情减少到 3 个控制路径,一个返回 Parallel ,一个返回 Perpendicular,一个返回 Double,而不是 12 个控制路径,每个控制路径有 4 个。我最初写它是因为它没有思考,老实说,思考它,这种方法减少了布尔语句的平均案例数。

我刚刚研究了如何通过打开evList[crysList[0]].crystalNum 来提高效率,我可以大大减少评估,得到这个:

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;
    }
}

问题仍然存在,有什么技巧可以缩短这个时间吗?更高效?更具可读性?

提前致谢!

【问题讨论】:

    标签: c++ if-statement switch-statement


    【解决方案1】:

    我认为您可以将几乎所有内容都移到一个简单的表格中,而无需进行单个表格查找。我没有详细研究过你的情况,但看起来像这样可以很好地完成这项工作:

    // 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;
        }
    }
    

    【讨论】:

    • 嗯,我喜欢这个想法,有什么反对使表格更小并从 a 和 b 的定义中减去偏移量?或者也许为每个检测器制作 4 个小型 4x4 命中表?我问是因为否则该表涵盖了很多不必要/无效的区域,并且我必须为该表编写的初始化列表变得巨大。对于 60x60,我必须写 3600 个值,对于 4 个 4x4 表,我必须写 64。
    • 当然。您可以在表格大小上进行很多优化。
    • 另请注意,表格是对称的,因此您可以比 4x4 表格节省 50%。然后,您将按 min(a,b) 和 max(a,b) 而不是直接按 a 和 b 进行索引。不过,寻址会稍微复杂一些。可能不值得努力但可行。
    • 是的,对称性可能不值得探索初始化列表的 64 个元素,这还不错,而且空间不是问题,不值得花时间交换 a 和 b,因此它们是按值顺序排列的。
    • 查看程序集,在我的第二次尝试中,最糟糕的情况是计算跳转表的索引、2 次绝对跳转、2 次逻辑比较、1 次移动和 2 次条件跳转。你把它打倒来计算一个跳转表索引、2 个绝对跳转、4 个各种类型的 mov、1 个加载有效地址和 1 个左移。有趣的是,我认为条件跳转在分支预测器上可能很难,但这种方法总体上似乎有更多的指令。我将不得不对这些进行基准测试。也就是说,您的版本更简洁,更易于阅读。再次感谢您的帮助!
    【解决方案2】:

    使其更短的一种解决方案是在比较/切换之前对晶体值进行排序

    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
    { ... }
    

    【讨论】:

    • 如果我查看我拍摄的第二个镜头的组件与这里的组件相比我的第二个镜头,这实际上必须在所有情况下执行更“丑陋”的指令,它肯定会帮助第一次尝试。不过感谢您的尝试。
    • 你可以使用std::minmax(需要C++11)。
    • @JamesMatta 优化编译器的输出很难理解,因此对人眼来说“丑陋”。然而,“丑陋”并不意味着它比更干净的版本更糟糕,因为可能会有更多的代码重新编排、内联、展开……使代码变大
    • @Luu Vinh Phuc 我可以阅读程序集 我理解输出当我说丑陋时我的意思是程序集有更多的条件跳转,这些条件跳转很昂贵。
    • 条件跳转不一定慢,只要分支是可预测的(并且 CPU 非常擅长)。事实上,使用条件移动甚至可能更慢
    【解决方案3】:

    结合了 4x4 查找表和按位运算的解决方案。

    说明: 这对于所有四种情况都是相同的,所以让我们看看det=10 的情况。

    在这种情况下,有趣的数字是 {40,41,42,43}。 如果我们以二进制表示形式查看这些数字,就会出现一个很好的模式。

    上面的数字是我们的掩码,由det*4 计算得出。 所以在这种情况下,我们的掩码是10*4=40

     0b101000 (40)  0b101000       0b101000       0b101000
    ^0b101000 (40) ^0b101001 (41) ^0b101010 (42) ^0b101011 (43)
    =0b000000      =0b000001      =0b000010      =0b000011
    

    在掩码和我们允许的数字之间进行异或 (^) 后,我们看到它们在集合 {0,1,2,3} 中都有一个值。 所以如果例如num1 ^ mask &lt; 4,这意味着只有最右边的两个位与mask 不同,或者num1 最多比mask 大3。 如果num1 &lt; mask 最右边的两个中至少有一些位会翻转,num1 ^ mask 将至少为 4。 因此,如果num1 ^ mask &lt; 4 为真,则num1 位于{40,41,42,43} 中。

    此外,由于它位于 {0,1,2,3} 中,我们还可以将其用作查找表中的索引。

    现在,如果前面的计算对于 num1num2 都是正确的,我们就有了查找表的组合索引。

    int checkDoubleGamma(int det){
        static const int hitTable[4][4] = {
            {Double, Parallel, Perpendicular, Double},
            {Parallel, Double, Double, Perpendicular},
            {Perpendicular, Double, Double, Parallel},
            {Double, Perpendicular, Parallel, Double}
        };
    
        const int num1 = evList[crysList[0]].crystalNum;
        const int num2 = evList[crysList[1]].crystalNum;
    
        switch(det) {
        case 10: //0b101000
        case 11: //0b101100
        case 13: //0b110100
        case 14: //0b111000
        {
            const unsigned int mask = 4 * det;
            const unsigned int a = num1 ^ mask;
            if(a < 4){
                const unsigned int b = num2 ^ mask;
                if(b < 4)
                    return hitTable[a][b];
            }
            return Double;
        }
        default:
            throw string("made it to default case in checkDoubleGamma switch statement, something is wrong");
            break;
        }
        //Never reaches here
    }
    

    编辑:这可以简化为 1x4 查找表

    如果我们查看a^b 的异或表,我们会发现它与我们的 4x4 查找表非常相似。

     ^ 00 01 10 11
    00 00 01 10 11
    01 01 00 11 10
    10 10 11 00 01 
    11 11 10 01 00
    

    这给了我们

    00,11 = Double
    01    = Parallel
    10    = Perpendicular 
    

    所以我们可以将旧的查找表缩减为 1x4 查找表,并使用 a^b 作为索引。

    通过查看a^b,我们看到它是num1^mask^num2^mask,它等于num1^num2,然后我们将使用它作为索引并保存一个异或指令。

    这仍然会检查 num2 是否在 {40,41,42,43} 内。如果num1mask 仅在最右边两位不同,num2num1 仅在最右边两位不同,则num2mask 仅在最右边两位不同。所以省略num2^mask 不会改变程序的行为。

    int checkDoubleGamma(int det){
        static const int hitTable[4] = {Double, Parallel, Perpendicular, Double};
    
        const int num1 = evList[crysList[0]].crystalNum;
        const int num2 = evList[crysList[1]].crystalNum;
    
        switch(det) {
        case 10: //0b101000
        case 11: //0b101100
        case 13: //0b110100
        case 14: //0b111000
        {
            const unsigned int mask = 4 * det;
            const unsigned int a = num1 ^ mask;
            if(a < 4){
                const unsigned int b = num1 ^ num2;
                if(b < 4)
                    return hitTable[b];
            }
            return Double;
        }
        default:
            throw string("made it to default case in checkDoubleGamma switch statement, something is wrong");
            break;
        }
        //Never reaches here
    }
    

    【讨论】:

      猜你喜欢
      • 1970-01-01
      • 2022-11-01
      • 1970-01-01
      • 1970-01-01
      • 1970-01-01
      • 1970-01-01
      • 1970-01-01
      • 1970-01-01
      • 1970-01-01
      相关资源
      最近更新 更多