优化三元运算符

Question

我遇到了其他人写的代码.推荐使用条件运算符还是常用？我觉得它不太可维护 - 或者只是我？有没有其他的写作方式？

  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;

Answer 1

那只是可怕的代码.

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

所以,你可以做什么？

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

而且要明确:三元运算符没有错.如果用得很明智,它通常会生成更容易消化的代码.尽管避免嵌套它们.如果代码清晰,我偶尔使用第二级,即使这样我也使用括号,所以我的可怜的大脑不需要做额外的循环来取消运算符优先级.

关心代码的读者.

Answer 2

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

检查代码,我的第一句话是,一系列三元运算符 - 就像所有代码一样 - 在充分格式化时更易读.

也就是说,我不确定我是否正确解析了OP的例子,这反对它.即使是传统的嵌套if-else结构也难以验证.这个表达式违反了基本的编程范式:Divide and Conquer.

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()
                &&  !isIgnorablePorotoViol()
                &&  !isIgnorablePciAbort()
            )
    {
        ret = req.slv_req[0].get_rsp_status();
    }

    return ret;
}

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

Answer 4

这是可怕的代码.

虽然通常需要使用单个表达式初始化变量(例如,我们可以创建它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?"};
  }();
}