79

我正在阅读 McConell 的Code Complete,他讨论了使用布尔变量来记录您的代码。例如,而不是:

if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) || 
   (elementIndex == lastElementIndex)){
       ...
}

他建议:

finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
   ...
}

这让我觉得这是合乎逻辑的、良好的实践和非常自我记录的。但是,我对是否开始经常使用这种技术犹豫不决,因为我几乎从未遇到过。也许仅仅因为稀有而令人困惑。但是,我的经验还不是很丰富,所以我很想听听程序员对这种技术的看法,我很想知道是否有人经常使用这种技术,或者在阅读代码时经常看到它。这是一个值得采用的约定/风格/技术吗?其他程序员会理解和欣赏它,还是认为它很奇怪?

4

14 回答 14

55

将一个过于嵌套和复杂的表达式拆分为分配给局部变量的更简单的子表达式,然后再组合在一起,这是一种非常普遍和流行的技术——完全独立于子表达式和/或整个表达式是布尔型还是几乎任何其他类型。如果名称选择得当,这种有品位的分解可以增加可读性,一个好的编译器应该可以毫不费力地生成与原始复杂表达式等效的代码。

一些本身没有“赋值”概念的语言,比如 Haskell,甚至引入了专门的结构来让你使用“给子表达式命名”技术(whereHaskell 中的子句)——似乎说明了一些相关技术的受欢迎程度!-)

于 2010-03-19T14:49:28.300 回答
16

我已经使用了它,尽管通常将布尔逻辑包装到可重用的方法中(如果从多个位置调用)。

它有助于提高可读性,当逻辑发生变化时,只需要在一个地方进行更改。

其他人会理解它并且不会觉得奇怪(除了那些只写过千行函数的人,即)。

于 2010-03-19T14:50:30.807 回答
9

我尽量做到这一点。当然,您正在使用“额外的一行”代码,但同时,您正在描述为什么要比较两个值。

在您的示例中,我查看代码并问自己“好吧,为什么看到该值的人小于 0?” 在第二个中,您清楚地告诉我,发生这种情况时,某些过程已经完成。在第二个中没有猜测你的意图是什么。

对我来说最重要的是当我看到这样的方法时: DoSomeMethod(true); 为什么它会自动设置为 true?它更具可读性

bool deleteOnCompletion = true;

DoSomeMethod(deleteOnCompletion);
于 2010-03-19T14:50:17.643 回答
5

提供的样本:

finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
   ...
}

也可以重写以使用方法,从而提高可读性并保留布尔逻辑(正如 Konrad 指出的那样):

if (IsFinished(elementIndex) || IsRepeatedEntry(elementIndex, lastElementIndex)){
   ...
}

...

private bool IsFinished(int elementIndex) {
    return ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
}

private bool IsRepeatedEntry(int elementIndex, int lastElementIndex) {
    return (elementIndex == lastElementIndex);
}

当然,这是有代价的,这是两种额外的方法。如果您经常这样做,它可能会使您的代码更具可读性,但您的类不那么透明。但话又说回来,您也可以将额外的方法移动到辅助类中。

于 2010-03-19T15:37:33.400 回答
3

我能看到这个错误的唯一方法是如果布尔片段没有一个有意义的名称并且无论如何选择了一个名称。

//No clue what the parts might mean.
if(price>0 && (customer.IsAlive || IsDay(Thursday)))

=>

first_condition = price>0
second_condition =customer.IsAlive || IsDay(Thursday)

//I'm still not enlightened.
if(first_condition && second_condition)

我指出这一点是因为制定诸如“注释所有代码”、“对所有超过 3 个部分的 if 条件使用命名布尔值”之类的规则是司空见惯的,只是为了获得以下类型的语义空的注释

i++; //increment i by adding 1 to i's previous value
于 2010-03-19T14:59:31.957 回答
2

如果表达式很复杂,那么我要么将其移至另一个返回booleg 的函数,isAnEveningInThePubAGoodIdea(dayOfWeek, sizeOfWorkLoad, amountOfSpareCash)要么重新考虑代码以便不需要这样复杂的表达式。

于 2010-03-19T15:11:10.180 回答
2

请记住,通过这种方式,您计算的内容超出了必要的范围。由于从代码中取出条件,您总是计算它们(没有短路)。

以便:

if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) || 
   (elementIndex == lastElementIndex)){
   ...
}

改造后变为:

if((elementIndex < 0) || (MAX_ELEMENTS < elementIndex) |
   (elementIndex == lastElementIndex)){
   ...
}

在大多数情况下这不是问题,但在某些情况下,这可能意味着更差的性能或其他问题,例如,在第二个表达式中,您假设第一个表达式失败。

于 2010-03-19T15:15:57.027 回答
2

通过做这个

finished = ((elementIndex < 0) || (MAX_ELEMENTS < elementIndex));
repeatedEntry = (elementIndex == lastElementIndex);
if(finished || repeatedEntry){
   ...
}

您从大脑中删除逻辑并将其放入代码中。现在程序知道你的意思了。
每当你命名某物时,你就赋予它物理表示。它存在。
您可以操纵和重用它。

您甚至可以将整个块定义为谓词:

bool ElementBlahBlah? (elementIndex, lastElementIndex);

并在该功能中做更多的事情(稍后)。

于 2010-03-19T15:09:03.027 回答
2

我认为创建函数/方法而不是临时变量更好。这种方式的可读性也提高了,因为方法变得更短了。Martin Fowler 的书 Refactoring 对提高代码质量有很好的建议。与您的特定示例相关的重构称为“用查询替换临时”和“提取方法”。

于 2010-03-19T16:00:01.243 回答
2

就个人而言,我认为这是一个很好的做法。它对代码执行的影响很小,但是如果使用得当,它可以提供的清晰度对于以后维护代码是非常宝贵的。

于 2010-03-19T16:04:12.623 回答
1

如果该方法需要成功通知:(c# 中的示例)我喜欢使用

bool success = false;

开始。代码是错误的,直到我将其更改为:

success = true;

然后在最后:

return success;
于 2010-03-23T00:18:09.950 回答
0

I think, it depends on what style you / your team prefer. "Introduce variable" refactoring could be useful, but sometimes not :)

And I should disagree with Kevin in his previous post. His example, I suppose, usable in case, when introduced variable can be changed, but introducing it only for one static boolean is useless, because we have parameter name in a method declaration, so why duplicate it in code?

for example:

void DoSomeMethod(boolean needDelete) { ... }

// useful
boolean deleteOnCompletion = true;
if ( someCondition ) {
    deleteOnCompletion = false;
}
DoSomeMethod(deleteOnCompletion);

// useless
boolean shouldNotDelete = false;
DoSomeMethod(shouldNotDelete);
于 2010-03-19T14:54:02.873 回答
0

我很少创建单独的变量。当测试变得复杂时,我会做的是嵌套 IF 并添加注释。喜欢

boolean processElement=false;
if (elementIndex < 0) // Do we have a valid element?
{
  processElement=true;
}
else if (elementIndex==lastElementIndex) // Is it the one we want?
{
  processElement=true;
}
if (processElement)
...

这种技术公认的缺陷是下一个出现的程序员可能会更改逻辑但不会费心更新评论。我想这是一个普遍的问题,但我有很多次看到一条评论说“验证客户 ID”,下一行是检查零件号或类似的东西,我想知道客户在哪里id进来了。

于 2010-03-19T17:38:50.667 回答
0

根据我的经验,我经常回到一些旧剧本并想知道“我当时到底在想什么?”。例如:

Math.p = function Math_p(a) {
    var r = 1, b = [], m = Math;
    a = m.js.copy(arguments);
    while (a.length) {
        b = b.concat(a.shift());
    }
    while (b.length) {
        r *= b.shift();
    }
    return r;
};

这不像:

/**
 * An extension to the Math object that accepts Arrays or Numbers
 * as an argument and returns the product of all numbers.
 * @param(Array) a A Number or an Array of numbers.
 * @return(Number) Returns the product of all numbers.
 */
Math.product = function Math_product(a) {
    var product = 1, numbers = [];
    a = argumentsToArray(arguments);
    while (a.length) {
        numbers = numbers.concat(a.shift());
    }
    while (numbers.length) {
        product *= numbers.shift();
    }
    return product;
};
于 2010-03-19T17:30:22.217 回答