优化三元运算符

这段代码太可怕了。

• 格式很差。我没有看到表达式的层次结构。
• 即使它有很好的格式，表达式也会太复杂，无法用人眼快速解析。
• 意图不明确。这些条件的目的是什么?

那么你能做什么呢?

• 使用条件语句(if)
• 提取子表达式，并将它们存储在变量中。从重构目录中查看这个不错的例子。
• 使用辅助函数。如果逻辑复杂，请使用早期的return。没有人喜欢深缩进。
• 最重要的是，给所有东西起一个有意义的名字。意图应该是明确的，为什么有些东西必须计算。

明确一点:三元操作符没有问题。如果使用得当，它通常会生成更容易消化的代码。但是要避免嵌套。如果代码非常清晰，我偶尔会使用第二层，即使这样，我也会使用括号，这样我可怜的大脑就不必做额外的循环来解密操作符优先级。

关心代码的读者。

也许这是在设备驱动程序的消息循环中，而最初的编码器(可能是10年前)不希望在代码中跳转。我希望他验证了他的编译器没有实现带跳转的三元运算符!

检查代码，我的第一个评论是，像所有代码一样，如果格式化得当，三元操作符序列具有更好的可读性。

也就是说，我不确定我正确地解析了OP的例子，这是反对它的。即使是传统的嵌套if-else构造也很难验证。这个表达式违背了基本的编程范例:分而治之。

req.security_violation
?   dis_prot_viol_rsp && is_mstr
    ?   uvc_pkg::MRSP_OKAY
    :   uvc_pkg::MRSP_PROTVIOL
:   req.slv_req.size()
    ?       is_mst_abort_rsp && dis_mst_abort_rsp
        ||      req.slv_req[0].get_rsp_status()==uvc_pkg::MRSP_PROTVIOL
            &&  dis_prot_viol_rsp
        ||  is_mst_abort_rsp && req.is_pci_config_req() && dis_pcicfg_mst_abort_rsp
        ?   uvc_pkg::MRSP_OKAY
        : req.slv_req[0].get_rsp_status()
    : uvc_pkg::MRSP_OKAY;

我想检查代码在重构时的样子。它确实不短，但我喜欢说话函数名称使意图更清晰的方式(当然我在这里猜测)。在某种程度上，这是伪代码，因为变量名可能不是全局的，所以函数必须有参数，这再次使代码不那么清晰。但也许参数可以是一个单一的指针状态或请求结构或这样(像dis_prot_viol_rsp的值已被提取)。在组合不同的条件时是否使用三进制还有待讨论。我觉得它常常很优雅。

bool ismStrProtoViol()
{
    return dis_prot_viol_rsp && is_mstr;
}
bool isIgnorableAbort()
{
    return is_mst_abort_rsp && dis_mst_abort_rsp;
}
bool isIgnorablePciAbort()
{
    return is_mst_abort_rsp && req.is_pci_config_req() && dis_pcicfg_mst_abort_rsp;
}
bool isIgnorableProtoViol()
{
    return req.slv_req[0].get_rsp_status()==uvc_pkg::MRSP_PROTVIOL &&  dis_prot_viol_rsp;
}

eStatus getRspStatus()
{
    eStatus ret;
    if( req.security_violation )
    {
        ret = ismStrProtoViol() ?  uvc_pkg::MRSP_OKAY : uvc_pkg::MRSP_PROTVIOL;
    }
    else if(  req.slv_req.size() )
    {
        ret =       isIgnorableAbort()
                ||  isIgnorableProtoViol()
                ||  isIgnorablePciAbort()
            ? uvc_pkg::MRSP_OKAY
            : req.slv_req[0].get_rsp_status();
    else
    {
        ret = uvc_pkg::MRSP_OKAY;
    }
    return ret;
}

最后，我们可以利用uvc_pkg::MRSP_OKAY是默认的并且只在某些情况下被覆盖的事实。这就消除了一个分支。看看经过一些修饰后，代码的推理是如何清晰可见的:如果它没有违反安全，请仔细检查实际的请求状态，减去空请求和可忽略的中止。

eStatus getRspStatus()
{
    eStatus ret = uvc_pkg::MRSP_OKAY;
    if( req.security_violation )
    {
        ret = ismStrProtoViol() ? uvc_pkg::MRSP_OKAY : uvc_pkg::MRSP_PROTVIOL;
    }
    else if(        req.slv_req.size()
                &&  !isIgnorableAbort()
                &&  !isIgnorableProtoViol()
                &&  !isIgnorablePciAbort()
            )
    {
        ret = req.slv_req[0].get_rsp_status();
    }
    return ret;
}

真难看。我把它分解成if和else来看看它在做什么。不太好读，但我还是想贴出来。希望其他人有更好的解决方案。但要回答你的问题，不要用那么复杂的三元。没有人想做我刚刚做的事情来弄清楚它在做什么。

if ( req.security_violation )
{
    if ( dis_prot_viol_rsp && is_mstr )
    {
        exp_rsp_status = uvc_pkg::MRSP_OKAY;
    }
    else
    {
        exp_rsp_status = uvc_pkg::MRSP_PROTVIOL;
    }
}
else if ( req.slv_req.size() )
{
    if ( ( is_mst_abort_rsp && dis_mst_abort_rsp ||
         ( req.slv_req[0].get_rsp_status() == uvc_pkg::MRSP_PROTVIOL && dis_prot_viol_rsp ) ||
         ( is_mst_abort_rsp && req.is_pci_config_req() && dis_pcicfg_mst_abort_rsp ) )
    {
        exp_rsp_status = uvc_pkg::MRSP_OKAY;
    }
    else
    {
        exp_rsp_status = req.slv_req[0].get_rsp_status();
    }
}
else
{
    exp_rsp_status = uvc_pkg::MRSP_OKAY
} 

这段代码太糟糕了。

虽然通常需要用单个表达式初始化变量(例如，我们可以使其为const)，但这不是编写这样代码的借口。您可以将复杂逻辑移到函数中，并调用它来初始化变量。

void
example(const int a, const int b)
{
  const auto mything = make_my_thing(a, b);
}

在c++ 11及以后的版本中，您还可以使用lambda来初始化变量。

void
example(const int a, const int b)
{
  const auto mything = [a, b](){
      if (a == b)
        return MyThing {"equal"};
      else if (a < b)
        return MyThing {"less"};
      else if (a > b)
        return MyThing {"greater"};
      else
        throw MyException {"How is this even possible?"};
  }();
}

其他人已经说过这段代码摘录有多糟糕，并给出了很好的解释。我将提供更多的原因，为什么这段代码是糟糕的:

1. 如果你考虑一个"if-else"来实现一个功能，那么很明显代码是多么复杂。

2. 很明显你的代码违反了单一职责原则，它告诉你:

   …一个类或模块应该有且只有一个改变的理由。

3. 单元测试将是一场噩梦，这是另一个危险信号。我敢打赌，你的同事甚至没有尝试为这段代码编写单元测试。

常用还是推荐?没有。

我也做了类似的事情，但我有我的理由:

是一个第三方C函数的参数。 那时候我对现代c++还不是很精通。
1. 我注释并格式化了它，因为我知道除了我之外还有人会读它…或者我需要知道几年后它在做什么。

2. 这是DEBUG CODE，永远不会进入发行版。

   textprintf_ex(gw->GetBackBuffer(), font, 0, 16, WHITE, -1, "BUTTON: %s",
                          //If...                        Then Display...
                     (ButtonClicked(Buttons[STOP])    ?  "STOP"
                   : (ButtonClicked(Buttons[AUTO])    ?  "AUTO" 
                   : (ButtonClicked(Buttons[TICK])    ?  "TICK"
                   : (ButtonClicked(Buttons[BLOCK])   ?  "BLOCK"
                   : (ButtonClicked(Buttons[BOAT])    ?  "BOAT"
                   : (ButtonClicked(Buttons[BLINKER]) ?  "BLINKER"
                   : (ButtonClicked(Buttons[GLIDER])  ?  "GLIDER"
                   : (ButtonClicked(Buttons[SHIP])    ?  "SHIP"
                   : (ButtonClicked(Buttons[GUN])     ?  "GUN"
                   : (ButtonClicked(Buttons[PULSAR])  ?  "PULSAR"
                   : (ButtonClicked(Buttons[RESET])   ?  "RESET"
                   :  /*Nothing was clicked*/            "NONE"
                   )))))))))))
                );
   

我没有使用if-else链的唯一原因是它会使代码变得庞大且难以理解，因为我所需要做的就是在屏幕上打印一个单词。