清洁代码 - 我应该将变量1更改为常量吗?

为了避免幻数,我们经常听到我们应该给变量一个有意义的名字。如:

//THIS CODE COMES FROM THE CLEAN CODE BOOK
for (int j = 0; j < 34; j++) {
    s += (t[j] * 4)/5;
}

-------------------- Change to --------------------

int realDaysPerIdealDay = 4;
const int WORK_DAYS_PER_WEEK = 5;
int sum = 0;
for (int j = 0; j < NUMBER_OF_TASKS; j++) {
    int realTaskDays = taskEstimate[j] * realDaysPerIdealDay;
    int realTaskWeeks = (realdays/WORK_DAYS_PER_WEEK);
    sum += realTaskWeeks;
}

我有一个像这样的虚拟方法:

解释:我想我有一份服务清单,默认情况下,我们只花5美元购买食物,但当我们有一个以上的人时,我们需要买水和食物,我们必须花更多的钱,可能是6美元。我会更改我的代码,请关注变量1 ,我的问题。

public int getMoneyByPersons(){
    if(persons.size() == 1){ 
       //TODO - return money for one person
    } else {
       //TODO - calculate and return money for people.
    }

}

当我让我的朋友们查看我的代码时,有人说给出值1的名称会产生更清晰的代码,而另一个说我们不需要常量名称,因为这个值本身就是有意义的。

So, my question is Should I give a name for the value 1? When is a value a magic number and when is it not? How can I distinguish context to choose the best solution?

6
也许这个逻辑更适合moneyService.getMoney()?在一个人的情况下,你是否需要打电话给getMoney?但我同意1清楚的一般情绪。魔术数字是你不得不挠头询问程序员如何到达那个数字的数字..即 if(getErrorCode()。equals(4095))...
额外 作者 Justin Poliey,
@selmaohneh我很确定清洁代码会结合你的评论和Neil's:将if check移到'moneyService',但这只是在其他地方移动OP的问题 - 在那里,你应该然后为了清晰分配bool变量'isSinglePerson',然后再将其用于实际决策。
额外 作者 R. Schmitz,
它没有比1.更清楚。我会改变“大小”到“计数”。而moneyService实际上是愚蠢的,它应该能够根据人员收集来决定返还多少钱。因此,我会将人员传递给getMoney并让它解决任何特殊情况。
额外 作者 Martin Maat,
来自哪里,它描述的是什么?您的代码没有任何评论,因此很难猜测它正在做什么。
额外 作者 Hubert Grzeskowiak,
您还可以将if子句中的逻辑表达式提取到新方法。也许称之为IsSinglePerson()。这样你提取的内容不仅仅是一个变量,而且使if子句更具可读性......
额外 作者 user1875642,
对不起,我更新了我的问题以便于理解。
额外 作者 Jacky,

5 答案

不。在这个例子中,1是完全有意义的。

但是,如果persons.size()为零呢?似乎很奇怪 persons.getMoney()适用于0和2但不适用于1。

16
额外
我同意1是有意义的。谢谢,顺便说一句,我更新了我的问题。你可以再次看到它以获得我的想法。
额外 作者 Jacky,

为什么一段代码包含特定的字面值?

  • 此值在问题域中具有特殊含义吗?
  • 或者这个值只是实现细节,其中该值是周围代码的直接结果?

如果文字值具有从上下文中不清楚的含义,则是,通过常量或变量赋予该值一个名称是有帮助的。之后,当忘记原始上下文时,具有有意义的变量名称的代码将更易于维护。请记住,代码的受众主要不是编译器(编译器会很乐意使用可怕的代码),而是代码的未来维护者 - 如果代码有点自我解释,他们会很感激。

  • 在您的第一个示例中, 3445 等文字的含义在上下文中并不明显。相反,其中一些值在您的问题域中具有特殊含义。因此,给他们起名字很好。

  • 在第二个例子中,文字 1 的含义在上下文中非常清楚。引入名称没有帮助。

实际上,为明显的值引入名称也可能很糟糕,因为它隐藏了实际值。

  • 如果命名值发生变化或不正确,这可能会掩盖错误,特别是如果相同的变量在不相关的代码段中重复使用。

  • 一段代码也可能适用于特定值,但在一般情况下可能不正确。通过引入不必要的抽象,代码不再明显正确。

“明显”文字没有大小限制,因为这完全取决于上下文。例如。文本 1024 在文件大小计算的上下文中可能是完全明显的,或者在散列函数的上下文中使用文字 31 ,或者文字 padding:0.5在CSS样式表的上下文中的em

11
额外

这段代码有几个问题,顺便说一下,可以这样缩短:

public List getMoneyByPersons() {
    return persons.size() == 1 ?
        moneyService.getMoneyIfHasOnePerson() :
        moneyService.getMoney(); 
}
  1. It is unclear why one person is a special case. I suppose that there is a specific business rule which tells that getting money from one person is radically different from getting money from several persons. However, I have to go and look inside both getMoneyIfHasOnePerson and getMoney, hoping to understand why are there distinct cases.

  2. The name getMoneyIfHasOnePerson doesn't look right. From the name, I would expect the method to check if there is a single person and, if this is the case, get money from him; otherwise, do nothing. From your code, this is not what is happening (or you're doing the condition twice).

  3. Is there any reason to return a List rather than a collection?

回到你的问题,因为目前还不清楚为什么对一个人有特殊待遇,数字一应该被一个常数替换,除非还有另一种方法要做规则明确。在这里,一个与任何其他魔术数字没有太大差别。您可以制定业务规则,告知特殊处理适用于一人,两人或三人,或仅适用于十二人以上。

如何区分上下文以选择最佳解决方案?

你做任何使你的代码更明确的事情。

例1

想象一下以下代码:

if (sequence.size() == 0) {
    return null;
}

return this.processSequence(sequence);

零在这里是一个神奇的价值吗?代码非常清楚:如果序列中没有元素,则不要处理它并返回特殊值。但是这段代码也可以像这样重写:

if (sequence.isEmpty()) {
    return null;
}

return this.processSequence(sequence);

在这里,不再是常数,代码更清晰。

例2

再拿一段代码:

const result = Math.round(input * 1000)/1000;

理解它在没有 round(value,precision)重载的JavaScript等语言中的作用并不需要太多时间。

现在,如果你想引入一个常量,它将如何调用?您可以获得的最接近的术语是 Precision 。所以:

const precision = 1000;
const result = Math.round(input * precision)/precision;

它是否提高了可读性?有可能。这里,常量的值相当有限,您可能会问自己是否真的需要执行重构。这里的好处是,现在,精度只被声明一次,所以如果它发生变化,你就不会犯下如下错误:

const result = Math.round(input * 100)/1000;

更改一个位置的值,而忘记在另一个位置执行此操作。

例3

从这些示例中,您可能会认为数字应该被每种情况中的常量替换。这不是真的。在某些情况下,使用常量不会导致代码改进。

采取以下代码:

class Point
{
    ...
    public void Reset()
    {
        x, y = (0, 0);
    }
}

如果你试图用变量替换零,那么难点就是找到一个有意义的名字。你怎么称呼它? <�代码> ZeroPosition </代码>? <�代码>基</代码>? <�代码>默认</代码>?在这里引入常量不会以任何方式增强代码。它会让它稍微长一些,就这样。

然而,这种情况很少见。因此,只要您在代码中找到一个数字,就要努力找到如何重构代码。问问自己这个号码是否有商业意义。如果是,则必须使用常量。如果不是,你会如何命名?如果你找到一个有意义的名字,那就太好了。如果没有,你很可能找到一个不需要常量的情况。

4
额外
我会添加3a:不要在变量名,使用金额,余额等中使用单词money。一堆钱没有意义,金额或余额的集合确实如此。 (好吧,我确实有一小部分外国货币(或者更确切地说是硬币),但这是一个不同的非编程问题)。
额外 作者 Bent,

您可以创建一个函数,它接受一个参数并返回四次除以五的时间,它在仍然使用第一个示例的同时提供了干净的别名。

我只是提供我自己常用的策略,但也许我也会学到一些东西。

我在想什么。

// Just an example name  
function normalize_task_value(task) {  
    return (task * 4)/5;  
}  

// Is it possible to just use tasks.length or something like that?  
// this NUMBER_OF_TASKS is the only one thats actually tricky to   
// understand right now how it plays its role.  

function normalize_all_task_values(tasks, accumulator) {  
    for (int i = 0; i < NUMBER_OF_TASKS; i++) {  
        accumulator += normalize_task_value(tasks[i]);  
}  

对不起,如果我不在基地,但我只是一个JavaScript开发人员。

2
额外
但是这些值4和5意味着什么?如果其中一个需要更改,您是否能够在类似的上下文中找到所有使用该号码的地方并相应地更新这些位置?这是原始问题的症结所在。
额外 作者 Dee,
@ gnasher729:如果常量恰好出现在一个命名良好,记录良好的函数的源代码中,则可能不需要使用命名常量。一旦常量出现多次具有相同的含义,给该常量一个名称可确保您不必弄清楚文字42的所有实例是否意味着相同的事物。
额外 作者 Dee,
@BartvanIngenSchenau整个功能名称不正确。我更喜欢一个更好的函数名,一个带有spec的注释,该函数应该返回什么,以及一个注释为什么* 4/5实现了这一点。不需要常量名称。
额外 作者 gnasher729,
我认为第一个例子是非常自我解释,所以可能不是我能回答的。当你需要我写一个新的来完成所需要的时候,这个操作将来不应该改变。评论是我认为最有利于此目的的。这是一个减少我语言的一线电话。让外行绝对可以理解的重要性有多重要?
额外 作者 Brad,
在它的核心,如果您希望您可能需要更改值yes。但是,我会将其更改为const来表示变量。我不认为是这种情况,这应该只是一个居住在它上面的范围内的中间变量。我不想说出来并说一切都是函数的一个参数,但是如果它可以改变但很少我会把它作为你正在努力做出改变的那个函数或对象/结构范围的内部参数仅在全球范围内。
额外 作者 Brad,

这个数字1,可能是一个不同的数字?可能是2,或3,还是有逻辑上的原因,为什么它必须是1?如果它必须是1,那么使用1就可以了。否则,您可以定义常量。不要称那个恒定的一个。 (我已经看到了这一点)。

一分钟60秒 - 你需要一个常数吗?好吧,它 60秒,而不是50或70.而且每个人都知道。这样可以保持一定数量。

每页打印60个项目 - 这个数字可以很容易地为59或55或70.实际上,如果你改变字体大小,它可能会变成55或70.所以这里有一个有意义的常数被要求更多。

这也是一个意义有多清楚的问题。如果你写“分钟=秒/ 60”,那很清楚。如果你写“x = y/60”,那就不清楚了。某处必须有一些有意义的名字。

有一个绝对的规则:没有绝对的规则。通过练习,您将了解何时使用数字,以及何时使用命名常量。不要这样做是因为一本书这么说 - 直到你理解为什么会这么说。

0
额外