优化三元运算符
我遇到了别人写的代码。 推荐使用还是常用的条件操作符的用法？ 我觉得这是不可维护的 – 或者只是我？ 有没有其他的方式来写这个？
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; 这只是可怕的代码。
- 格式不正确。 我没有看到expression式的层次结构。
- 即使格式不错，expression也会太复杂，不能用人眼来快速parsing。
- 意图不清楚。 这些条件的目的是什么？
所以，你可以做什么？
- 使用条件语句（
if）。 - 提取子expression式，并将它们存储在variables中。 检查重构目录中的这个很好的例子。
- 使用助手function。 如果逻辑很复杂，则使用early
return。 没有人喜欢深刻的缩进。 - 最重要的是，给每件事一个有意义的名字。 意图应该清楚为什么必须计算。
而要清楚的是：三元运算符没有错。 如果使用得当，通常会产生更容易消化的代码。 避免嵌套它们。 如果代码很清晰，我偶尔会使用第二个级别，即使如此，我仍然使用括号，所以我可怜的大脑不需要额外的周期来解决运算符的优先级问题。
关心你的代码的读者。
也许这是在设备驱动程序的消息循环中，原来的编码器，可能在10年前，不想在代码中跳转。 我希望他证实他的编译器没有实现三元运算符的跳转！
检查代码，我的第一个评论是，一个三元运算符序列像所有的代码一样，在格式化的时候更好。
这就是说，我不确定我是否正确地parsing了OP的例子，这是不合理的。 即使是传统的嵌套if-else构造也很难validation。 这个expression式违反了基本的编程范式：分而治之。
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;
我想检查重构时代码的外观。 它肯定不会更短，但我喜欢说话的function名称使意图更清楚（当然我在这里猜到）。 这在一定程度上是伪代码，因为variables名可能不是全局的，所以函数必须有参数，使得代码不再清晰。 但是也许这个参数可能是一个指向状态或者请求结构的单个指针，或者是像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; }
什么丑陋的混乱。 我把它分解成if和else，看看它在做什么。 没有更多的可读性，但认为我会张贴反正。 希望别人有一个更优雅的解决scheme给你。 但要回答你的问题，不要使用复杂的三元组。 没有人愿意做我刚刚做的事情来弄清楚它在做什么。
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 }
这是可怕的代码。
虽然通常需要使用单个expression式来初始化一个variables（例如，所以我们可以使它成为const ），但是这样写代码是没有任何借口的。 您可以将复杂的逻辑移到一个函数中，并调用它来初始化variables。
void example(const int a, const int b) { const auto mything = make_my_thing(a, b); }
在C ++ 11及更高版本中，您也可以使用lambda来初始化一个variables。
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?"}; }(); }
其他人已经说过，代码摘录有多糟糕，有很好的解释。 我只会提供更多的原因，为什么代码是不好的：
-
如果你考虑一个“if-else”来实现一个特性，那么这个代码是多么的复杂。 在你的情况下，我甚至不能计算ifs的数量。
-
很明显，你的代码正在打破单一责任原则 ，它告诉：
…一个class级或模块应该有一个，只有一个理由来改变。
-
unit testing，这将是一场噩梦，这是另一个红旗。 我敢打赌，你的同事甚至不会为这段代码编写unit testing。
常用还是推荐？ 没有。
我做了类似的事情，但是我有我的理由：
- 这是第三方C函数的一个参数。
- 我当时并不熟悉现代C ++。
- 我评论和格式化了f ***，因为我知道除了我之外，还有人会读它…或者我需要知道几年后的情况。
-
这是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链的唯一原因是它会使代码变得非常难以遵循，因为我需要做的只是在屏幕上打印一个字。
