优化三元算子

Posted 2023-02-23

【中文标题】优化三元算子

【英文标题】：Optimize ternary operator

【发布时间】：2016-01-19 06:54:55

【问题描述】：

我遇到了其他人编写的这段代码。这种条件运算符的用法是推荐的还是常用的？我觉得它的可维护性较差 - 还是只有我一个人？有没有其他的写法？

  exp_rsp_status =  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;

【问题讨论】：

exp_rsp_status 默认可以构造和赋值吗？

相关：***.com/questions/18237432/… 或 ***.com/questions/19020092/…

你的问题集中在三元运算符上，而真正的问题是它太复杂了，不管是不是三元的。

我在这里删除了 [bad-code] 标签，因为它是一个“元标签”——一个描述问题是的标签，而不是它的内容。如果您想讨论它，您可以考虑询问on meta。谢谢！

不断回滚您不同意的编辑是没有建设性的。如果您想讨论编辑是否应该保留或离开，您可以在Meta Stack Overflow 上进行讨论，而不是进行拉锯战。

【参考方案1】：

这是糟糕的代码。

虽然通常希望使用单个表达式来初始化变量（例如，我们可以将其设为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?";
  ();

【讨论】：

我只想说你的第一个 sn-p 也是面向 C++11...auto 和所有 :)

很难理解如何使用这个答案。也许你应该用类似于问题中的代码的东西替换你的代码。

在这里使用 lambda 是一种很好的方式来混淆代码。

【参考方案2】：

这只是可怕的代码。

格式错误。我没有看到表达式的层次结构。 即使它具有良好的格式，表达式也会过于复杂，无法用人眼快速解析。 意图不明确。这些条件的目的是什么？

那你能做什么？

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

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

关心你的代码的读者。

【讨论】：

另外，如果使用三元运算是为了提高效率，您可以将这些条件拆分为（命名的）内联函数。

虽然很难弄清楚，但通过正确的格式和一堆子表达式提取，三元组可能没问题。我已经写了这么深的三元组，它们非常清楚，因为所有东西都有好名字，而且里面没有乱七八糟的数学。

@jpmc26 为什么？绝对有效，mac ie 支持18年！

@zakius 有些东西可能同时是有效的和可怕的。

【参考方案3】：

真是一团糟。我把它分解成 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
 

【讨论】：

我会将所有布尔逻辑分解为单独的、命名良好的变量，以便意图一目了然。

请将它重新打包成三元组，它的可读性差不多（即根本不可读），而且你可以更快地克服它。 :-D

是的，我认为这里的其他海报有正确的想法，为了清楚起见，将其分解为变量。我只是发布了这个，因为我想看看它在做什么，并认为它对其他人来说是有用的哈哈。

也许删除大逻辑表达式中所有不需要的括号？您已经通过换行符分隔不同的子句，因此不需要括号。另外，我认为它们不平衡（语法错误）。

@anatolyg：反对。括号从来都不是不必要的。作者或稍后进行调试的人必然会浪费时间引用运算符优先级表，和/或弄错。 ;-)

【参考方案4】：

也许这是在设备驱动程序的消息循环中，而最初的编码员，可能是 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;

【讨论】：

我会更进一步，尽早返回以完全摆脱 ret 变量...if(req.security_violation) return ismStrProtoViol() ? uvc_pkg::MRSP_OKAY : uvc_pkg::MRSP_PROTVIOL; ...。

这里不需要一堆函数。用存储在变量中的计算替换每个函数。

@MichaelAnderson 是的，这确实值得商榷。我通常倾向于从早期返回/多次返回开始，但是当代码发展时，只有一个条件，突然你有太多的情况需要跟踪。早期返回基本上会启动隐式else 分支（“如果我没有返回......”）。明确的 if/else 使这一点更加明显。--实际上，我还担心在编码练习中会因为不止一次的回报而被鄙视 ;-)。

不是 100% 确定（这是一个骗局），但我认为您重构的代码可能会导致旧代码不会调用 req.is_pci_config_req()。由于此类调用可能会产生副作用（时间或系统状态的更改），我认为我们不能保证新重构的代码将与旧代码类似地运行。影响范围从完全良性到灾难性。

看看您的答案的版本 1，我认为这些功能是一个更好的主意，因为它们保持了原始的短路行为。

【参考方案5】：

其他人已经说过该代码摘录有多糟糕，并有很好的解释。我将提供更多为什么该代码不好的原因：

如果您考虑一个“if-else”来实现一个功能，那么很明显该代码有多复杂。在你的情况下，我什至无法计算 if 的数量。

很明显，您的代码正在破坏single responsibility principle，它告诉：

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

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

【讨论】：

1。当然你可以数一数，有4个?s，即4个ifs（假设你没有看到|| &&是隐含的ifs）。 2。不清楚，因为您的链接说该原则适用于类，我们看不到给定表达式与解决其他问题的代码的分离程度。 3。 调试 使用我所知道的调试器肯定会令人厌烦，因为它们不提供允许一个单步执行表达式的工具，但是（黑盒）测试 应该独立于代码。

@PJTraill LOL 用于计算 IF 的数量（我认为 ?:、if-else 和布尔运算符是分支）。关于第 2 点，它说的是模块，如果您阅读这本书（在 Robert C Martin 的链接中），就会清楚它也可以应用于函数。

@PJTrailll 关于测试，我修改了答案。进行单元测试。

【参考方案6】：

常见还是推荐？没有。

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

这是第三方 C 函数的参数。 当时我并不精通现代 C++。 我评论并格式化了它，因为我知道除了我之外的其他人会阅读它......或者我需要知道几年后它在做什么。

调试代码永远不会发布。

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 链的唯一原因是它会使代码变得庞大且难以理解，因为我需要做的就是在屏幕上打印一个单词。

【讨论】：

我总是为此创建一个map<Button, string>。