它闻起来很臭,因为可能有很多情况可以对其进行编辑或改进。
大多数这些气味只是暗示可能有问题。它们本身不需要固定……(不过你应该研究一下。)
让我们继续...
与布尔值进行比较时,我们执行魔术转换并得到意想不到的结果。
TL;DR:不要与真实比较。要么你是真的,要么是假的,或者你不应该比较
许多语言将值转换为布尔交叉域。
#!/bin/bash if [ false ]; then echo "True" else echo "False" fi # this evaluates to true since # "false" is a non-empty string if [ false ] = true; then echo "True" else echo "False" fi # this also evaluates to true
#!/bin/bash if false ; then echo "True" else echo "False" fi # this evaluates to false
Linter 可以检查明确的比较和警告。
将许多非布尔值用作布尔值是一种常见的行业惯例。使用布尔值时我们应该非常严格。
Code Smell 69 - 生活大爆炸(JavaScript 荒谬的铸件)
如果它不起作用,那么它不起作用的速度有多快都没关系。
-米奇·拉维拉
嵌套的 IF 和 Elses 很难阅读和测试
TL;DR:避免嵌套 IF。更好:避免所有 IF
在过程代码中,很常见的是复杂的嵌套 if。此解决方案与脚本相关的程度高于面向对象的编程。
if (actualIndex < totalItems) { if (product[actualIndex].Name.Contains("arrow")) { do { if (product[actualIndex].price == null) { // handle no price } else { if (!(product[actualIndex].priceIsCurrent())) { // add price } else { if (!hasDiscount) { // handle discount } else { // etc } } } actualIndex++; } while (actualIndex < totalCounf && totalPrice < wallet.money); } else actualIndex++; } return actualIndex; }
foreach (products as currentProduct) addPriceIfDefined(currentProduct) addPriceIfDefined() { //Several extracts }
由于许多 linter 可以解析树,我们可以在编译时检查嵌套级别。
按照鲍勃叔叔的建议,我们应该让代码比我们找到的更干净。
重构这个问题很容易。
Code Smell 36 - Switch/case/elseif/else/if 语句
软件工程的目的是控制复杂性,而不是创造复杂性。
- 帕梅拉扎夫
调用我们自己的访问器方法似乎是一个很好的封装想法。但事实并非如此。
TL;DR:不要使用 setter 和 getter,即使是私人使用
使用双重封装是 90 年代的标准程序。
即使是为了私人使用,我们也想隐藏实现细节。
当太多的功能依赖于数据结构和意外实现时,这隐藏了另一种气味。
例如,我们可以改变一个对象的内部表示并依赖它的外部协议。
成本/收益不值得。
contract MessageContract { string message = "Let's trade"; function getMessage() public constant returns(string) { return message; } function setMessage(string newMessage) public { message = newMessage; } function sendMessage() public constant { this.send(this.getMessage()); // We can access property but make a self call instead } }
contract MessageContract { string message = "Let's trade"; function sendMessage() public constant { this.send(message); } }
我们可以推断 getter 和 setter 并检查它们是否是从同一个对象调用的。
双重封装是一种保护意外实现的流行想法,但它暴露的比保护的多。
雷·轩尼诗 ( Ray Hennessy ) 在Unsplash上拍摄的照片
封装变化的概念。
-埃里希伽玛
断言布尔值会使错误跟踪更加困难。
TL;DR:除非您检查布尔值,否则不要断言 true
当断言一个布尔值时,我们的测试引擎对我们帮助不大。
他们只是告诉我们有些事情失败了。
错误跟踪变得更加困难。
<? final class RangeUnitTest extends TestCase { function testValidOffset() { $range = new Range(1, 1); $offset = $range->offset(); $this->assertTrue(10 == $offset); // No functional essential description :( // Accidental description provided by tests is very bad } } // When failing Unit framework will show us // // 1 Test, 1 failed // Failing asserting true matches expected false :( // () <-- no business description :( // // <Click to see difference> - Two booleans // (and a diff comparator will show us two booleans)
<? final class RangeUnitTest extends TestCase { function testValidOffset() { $range = new Range(1, 1); $offset = $range->offset(); $this->assertEquals(10, $offset, 'All pages must have 10 as offset'); // Expected value should always be first argument // We add a functional essential description // to complement accidental description provided by tests } } // When failing Unit framework will show us // // 1 Test, 1 failed // Failing asserting 0 matches expected 10 // All pages must have 10 as offset <-- business description // // <Click to see difference> // (and a diff comparator will help us and it will be a great help // for complex objects like objects or jsons)
如果我们在设置此条件后检查布尔值,一些 linter 会警告我们。
我们需要将其更改为更具体的检查。
尝试重写您的布尔断言,您将更快地修复故障。
照片由Joël de Vriend在Unsplash上拍摄
我终于明白了“向上兼容”是什么意思。这意味着我们可以保留我们所有的旧错误。
——丹尼·范·塔塞尔
使用专业且有意义的名称
TL;DR:不要随意或冒犯
我们的职业有创造性的一面。
有时我们会感到无聊并试图变得有趣。
function erradicateAndMurderAllCustomers(); // unprofessional and offensive
function deleteAllCustomers(); // more declarative and professional
我们可以有一个禁用词列表。
我们还可以在代码审查中检查它们。
名称是上下文相关的,因此对于自动 linter 来说这将是一项艰巨的任务。
命名约定应该是通用的,不应包含文化术语。
在代码中命名事物的方式要专业。
不要通过给变量起一个愚蠢的名字来试图成为喜剧演员。
您应该编写生产代码,以便未来的软件开发人员(甚至您)应该很容易理解。
斯图尔特·蒙罗 ( Stewart Munro)在Unsplash上拍摄的照片
这种“用户是白痴,被功能所迷惑”的 Gnome 心态是一种病。如果您认为您的用户是白痴,那么只有白痴才会使用它。
-莱纳斯·托瓦兹
这就是现在的全部……
下一篇文章将解释另外 5 种代码异味!