优化三元运算符
Optimize ternary operator
我看到了别人写的这段代码。条件运算符的这种用法是推荐的还是常用的?我觉得它不太容易维护 - 或者只是我?有没有其他的写法?
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;
这是糟糕的代码。
虽然通常希望用单个表达式初始化变量(例如,我们可以使它成为 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?"};
}();
}
那真是糟糕的代码。
- 格式错误。我没有看到表达式的层次结构。
- 即使它具有良好的格式,表达式也会太复杂而无法用人眼快速解析。
- 意图不明。这些条件的目的是什么?
那你能做什么?
- 使用条件语句(
if
)。
- 提取子表达式,并将它们存储在变量中。检查重构目录中的 this 个不错的示例。
- 使用辅助函数。如果逻辑复杂,请使用早期的
return
s。没有人喜欢深压痕。
- 最重要的是,给所有东西起一个有意义的名字。意图应该很清楚为什么必须计算某些东西。
需要说明的是:三元运算符没有任何问题。如果谨慎使用,它通常会生成更容易理解的代码。但是要避免嵌套它们。如果代码 crystal 清晰,我偶尔会使用第二个级别,即使那样我也会使用括号,这样我可怜的大脑就不必做额外的循环来破译运算符的优先级。
关心代码的读者。
真是一团糟。我把它分解成 if 和 else 只是为了看看它在做什么。可读性不高,但我想我还是 post 了。希望其他人为您提供更优雅的解决方案。但是要回答你的问题,不要使用那么复杂的三元组。没有人想做我刚刚做的事情来弄清楚它在做什么。
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
}
也许这是在设备驱动程序的消息循环中,原始编码器可能在 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" 只实现一个功能,那么很明显该代码有多么复杂。在你的情况下,我什至无法计算 ifs 的数量。
很明显,您的代码正在破坏 single responsibility principle,它告诉:
...a class or module should have one, and only one, reason to change.
单元测试将是一场噩梦,这是另一个危险信号。我敢打赌你的同事甚至没有尝试为那段代码编写单元测试。
常用还是推荐?编号
我做了类似的事情,但我有我的理由:
- 这是第三方 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 链的唯一原因是它会使代码变得庞大且难以理解,因为我需要做的就是在屏幕上打印一个字。
我看到了别人写的这段代码。条件运算符的这种用法是推荐的还是常用的?我觉得它不太容易维护 - 或者只是我?有没有其他的写法?
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;
这是糟糕的代码。
虽然通常希望用单个表达式初始化变量(例如,我们可以使它成为 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?"};
}();
}
那真是糟糕的代码。
- 格式错误。我没有看到表达式的层次结构。
- 即使它具有良好的格式,表达式也会太复杂而无法用人眼快速解析。
- 意图不明。这些条件的目的是什么?
那你能做什么?
- 使用条件语句(
if
)。 - 提取子表达式,并将它们存储在变量中。检查重构目录中的 this 个不错的示例。
- 使用辅助函数。如果逻辑复杂,请使用早期的
return
s。没有人喜欢深压痕。 - 最重要的是,给所有东西起一个有意义的名字。意图应该很清楚为什么必须计算某些东西。
需要说明的是:三元运算符没有任何问题。如果谨慎使用,它通常会生成更容易理解的代码。但是要避免嵌套它们。如果代码 crystal 清晰,我偶尔会使用第二个级别,即使那样我也会使用括号,这样我可怜的大脑就不必做额外的循环来破译运算符的优先级。
关心代码的读者。
真是一团糟。我把它分解成 if 和 else 只是为了看看它在做什么。可读性不高,但我想我还是 post 了。希望其他人为您提供更优雅的解决方案。但是要回答你的问题,不要使用那么复杂的三元组。没有人想做我刚刚做的事情来弄清楚它在做什么。
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
}
也许这是在设备驱动程序的消息循环中,原始编码器可能在 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" 只实现一个功能,那么很明显该代码有多么复杂。在你的情况下,我什至无法计算 ifs 的数量。
很明显,您的代码正在破坏 single responsibility principle,它告诉:
...a class or module should have one, and only one, reason to change.
单元测试将是一场噩梦,这是另一个危险信号。我敢打赌你的同事甚至没有尝试为那段代码编写单元测试。
常用还是推荐?编号
我做了类似的事情,但我有我的理由:
- 这是第三方 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 链的唯一原因是它会使代码变得庞大且难以理解,因为我需要做的就是在屏幕上打印一个字。