においがするのは、編集または改善できる場合が多いためです。
これらの臭いのほとんどは、何かが間違っている可能性があることを示しているだけです。それらはそれ自体を修正する必要はありません…(ただし、調べておく必要があります。)
続けましょう...
ブール値と比較すると、魔法のキャストを実行し、予期しない結果が得られます。
TL;DR: true と比較しないでください。あなたが正しいか、間違っているか、比較するべきではないかのどちらかです
多くの言語は、値をブール交差ドメインにキャストします。
#!/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
リンターは、明示的な比較と警告をチェックできます。
多くの非ブール値をブール値として使用することは、一般的な業界慣行です。ブール値を使用するときは、非常に厳密にする必要があります。
Code Smell 69 - ビッグバン (JavaScript とんでもないキャスティング)
うまくいかない場合は、どれだけ速くうまくいかなくても構いません。
- ミッチ・ラヴェラ
ネストされた IF と Elses は、読み取りとテストが非常に困難です。
TL;DR: ネストされた IF は避けてください。さらに良いこと: ALL 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 }
多くのリンターはツリーを解析できるため、コンパイル時にネストレベルをチェックできます。
ボブおじさんのアドバイスに従って、コードを見つけたときよりもクリーンなままにしておく必要があります。
この問題のリファクタリングは簡単です。
Code Smell 36 - Switch/case/elseif/else/if ステートメント
ソフトウェア エンジニアリングの目的は、複雑さを作成することではなく、制御することです。
- パメラ・ザベ
独自のアクセサ メソッドを呼び出すことは、カプセル化の良いアイデアに思えるかもしれません。そうではありません。
TL;DR: 私的な使用であっても、セッターとゲッターを使用しないでください
二重カプセル化の使用は、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); } }
ゲッターとセッターを推測し、それらが同じオブジェクトから呼び出されているかどうかを確認できます。
二重カプセル化は、偶発的な実装を保護するための流行のアイデアでしたが、保護以上のものを公開しました。
変化する概念をカプセル化します。
-エーリッヒ・ガンマ
ブール値に対してアサートすると、エラーの追跡がより困難になります。
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)
この条件を設定した後にブール値をチェックすると、一部のリンターは警告を発します。
より具体的なチェックに変更する必要があります。
ブール アサーションを書き直してみてください。そうすれば、失敗をはるかに迅速に修正できます。
UnsplashのJoëldeVriendによる写真
「上位互換性」の意味をようやく学びました。これは、過去の過ちをすべて保持できることを意味します。
-デニー・ヴァン・タッセル
専門的で意味のある名前を使用する
TL;DR: 非公式または攻撃的にならないでください
私たちの職業には創造的な側面があります。
時々私たちは退屈して、面白くしようとします。
function erradicateAndMurderAllCustomers(); // unprofessional and offensive
function deleteAllCustomers(); // more declarative and professional
禁止語のリストを取得できます。
コードレビューでも確認できます。
名前は文脈に依存するため、自動リンターにとっては難しい作業です。
命名規則は一般的なものにする必要があり、文化的な専門用語を含めないでください。
コード内で名前を付ける方法については、プロ意識を持ってください。
変数にばかげた名前を付けてコメディアンになろうとしないでください。
将来のソフトウェア開発者 (あなたも) が簡単に理解できるように、製品コードを作成する必要があります。
この「ユーザーは馬鹿で、機能性に惑わされている」という Gnome のメンタリティは病気です。ユーザーがばかだと思うなら、ばかだけがそれを使用します。
-ライナス・トーバルズ
そして、それは今のところすべてです…
次の記事では、さらに5つのコードの匂いについて説明します!