LoginSignup
448
374

More than 3 years have passed since last update.

@yuu_j

こんなコードは嫌だ、古い書き方のコード駆逐したい（とりあえず９つ）

Last updated at Posted at 2021-02-18

時代は令和ぞ、何を書いとるんや

転職してきた若いプログラマが変なコード書いている。
どうやら前社の社内研修で教わったとのこと。
さて、何を教わったのだろうか。
※一応TypeScriptで書きましたが別にC#でも言えることです。
※CやC++やアセンブラのことは全く知らないので、そのあたり詳しい人は今どんな書き方か記事書いていただけると勉強になります。

１．変数名が雑

クラス、関数、変数、どれも命名は難しいものです。1
大体が英語で大変です。けど頑張ってわかりやすい名前つけるようにしています。
本読んで勉強してください。Google翻訳使ってください。

10行程度の短い関数ならretでもdataとか適当な名前でもいいけど
長くなるようならちゃんと名前つけてるようにしたほうがいいです。
わかりやすい変数名をつけることでひと目で、その変数の役割が理解出来ます。


//　Goodってなんやねん！なにがGoodやねん！
public isGood : boolean { return true; }

public Piyo() : void {
    // この関数名もどうなんだ？GetClientでいいのか？
    data = this.getClientData();
    // 十行以上のコード

    // ちゃんと名前ついてないから上のコード見な何のデータかわからん！
    // clientDataとつけていたら顧客情報だとひと目で分かる
    if(data.result){
    }
}

２．無駄な省略

これも命名です。
古い習慣なんでしょうか？2 3
countをcntとか、managerをmgrとか、meetingをmtgとか
日常のメールとかに書く分にはいいと思いますがプログラムには書かないようにしたほうが良いと思います。
特に関数内のちょっとした変数ならともかく、
クラス名には用いないほうが良いです。

public Hoge() : void {
    // は？dtってなんやねん、DateTimeか？童○か？ 
    // ※誤解のないようにかいておきますがここはdataをdtと略している例
    let dt = this.getData();
    // 以降なんかの処理
}

３．無駄に()つけている

※この項は批判も多く、個人の考えによっているところがあります。
　正しいことを述べているのではなく、そのような意見もあるのだな程度に受け取ってください。

if文で無駄な()が多いと個人的に読みにくいです。

// このhogeとかpiyoの周りかっこいらんよね？
if((hoge == 'Hoge') && (piyo == 'piyo')){
    // なんかの処理
}

可読性を気にして、そんな書き方するぐらいなら
一回変数で受けたほうが読みやすいです。

const isHoge = hoge == 'Hoge';
const isPiyo = piyo == 'piyo';
if(isHoge && isPiyo){
    // なんかの処理
}

ただし、ifとorが混在するようなケースでは()をつけることが推奨されています。
Airbnb JavaScript Style Guide

Bad
// ぱっと見で評価順がわかりにく
if (a || b && c) {
  return d;
}

Good
// (a || b) && cと理解を間違えることがない
if (a || (b && c)) {
  return d;
}

４．変更するときに昔のコードをコメントアウトして残す

コード管理にgitなりSVNなり使っている職場ではコメントアウトして
残すメリットより可読性を損なう可能性のほうが高いので非推奨です。4

public commentOut() : void {
    const foo = new Foo();
    /* 2021//02/18 yuu_j HogeではなくFooを使うようになったので変更 
     * const hoge = new Hoge();
     * 昔の処理
     */
}

５．必要以上に広いスコープで変数宣言する

変数の宣言する場所考えてますか？
必要最小限のスコープで宣言したほうがいいです。5
そうすることで、変数を追う手間も少なくて済みます。

public getPrice(fuga : boolean, piyo : boolean) : number {
    let price = 0;
    // ここでitem宣言するとif文の外でも使われるのかと思われてしまう。
    const item = this.getItem();
    if(fuga){
        price += 100;
        if (piyo){
            // このif文の中でしかitem使ってないよね？この中で宣言しようね？
            price += item.Price;
        }
    }
    return price;
}

６．ハンガリアン

変数の宣言時に変数の頭に型がわかるように書くやつです。
個人的にはそこまで嫌いじゃないけど、下記のような記事で批判されており推奨されている書き方とは言えないと思います。6

間違ったコードは間違って見えるようにする - The Joel on Software Translation Project

(Joel on Softwareより) ハンガリアン記法の本当の意味 - 本当は怖いHPC

Joel on Software 日本語訳インデックス InternetArchive WayBackMachine | プログラミングアカデミー ゼロプラスワン

// number型だからnから始めるw
const nCount : number = 0

// string型だからsから始めるwww
const sName : string = ''

７．ヨーダ記法

定数をif文の左側にかくやつです。
コードの順番を変えることで可読性を損ないます。具体的な批判はWikipediaにも記載があります。7
ヨーダ記法 - Wikipedia

//アンチパターン
public badSample(price : number) : void{
    // 定数を左側に書くな！！
    if (0 == price) {
         // なんかの処理
    }
}

//推奨パターン
public goodSample(price : number) : void{
    // 定数は右に書け！！
    if (price == 0) {
         // なんかの処理
    }
}

８．言語として用意されている機能を使わない

これに関しては新人の勉強不足や、勉強しろ。8
特殊な外部ライブラリとにある機能ならともかく
デフォルトで用意されている機能使ったほうがいいと思います。
C#ならLinqを使うとかPythonでmathを使う等です。9

private _numbers : number [] = [1,2,3,4];

// アンチパターン
public badHasNumber(targetNumber : number){
    for(n in _numbers){
        if(n == targetNumber){
            return true;
        }
    }
    return false;
}

// 推奨パターン
public goodHasNumber(numbers: number[], targetNumber: number){
    return numbers.includes(targetNumber);
}

９．goto

とりあえずgoto必須の言語10以外でgoto使おうなんて考えないほうがいいと思います。
よっぽどのことがない限りC#で使ってたりしたら、ふざけてるとしか思わないです。11

最後に

この記事が新人プログラマが良くない習慣、良くない環境から抜け出す一助になればいいなと思います。
あとQiitaで表現されたコードってVS Codeとかで見るよりなんか見やすい気がしますね。フォントの問題？

@dairappa さんより、アンサー記事を書いていただきました。
本記事よりよっぽど丁寧でわかりやすく意味のあるものです。
re: こんなコードは嫌だ、古い書き方のコード駆逐したい

追記：2021/2/20

この記事を書いたときはかなり感情的になっていて失礼な表現が多々あると思います。
気分を悪くされた方には申し訳なく思います。
すみませんでした。

個人的に言いたかったのは経験豊富で知識豊富なエンジニアに対して、
あらゆる言語に、自分の経験豊富な言語の習慣をそのまま持ち込まないでほしいということです。
各言語には各言語らしい書き方があると思います。
各言語に合わせて、もしくは時流に合わせて変われないのであれば、
それは思い直していただきたいです。

私自身、未熟で学習中の身でございますので、見当外れな指摘もあると思います。
納得できる、もしくは一般的に批判されている部分だけでも
若いプログラミングを始めたばかりの方の参考になればと思う次第です。

追記:2021/02/22

いただいたコメントを参考に随所をなおしました。
修正後も随所に感情的で、きつい表現が残っていますが、
そのあたりはそれだけ思いがあるのです。
初めて見た人は、なんでこんなレベルの記事がバズったんだと思われるかと思いますが、
それは炎上して批判が集まったからです。
なぜ炎上したかというと、それは修正前の記事で『おじいコード』や『老害』等の不適切な表現や
随所に現れる偉そうな書き方があったからです。
どれだけひどかったかは編集履歴から確認できます。
ただ、自分勝手ではございますが、多くにご意見をいただき個人的にはとても勉強になりました。
『「藪をつついて蛇を出す」を地でいく事例ですね』というコメントもいただき、そのとおりですが
その一方で「棚からぼた餅」だったなとも思っています。
多くのアドバイス、ご指摘をいただきありがとうございます。

  1. コーディングの勉強に際して評判の良いリーダブルコードでも2章に渡って命名について書いてありました。それぐらい難しいものだと思います。いやそれ以上か... ↩

  2. 『なお、世の中にはgolangのようにローカル変数を1文字で表現し、可読性を高める宗派もあります。』、『最近人気のGo言語では、「変数名はできるだけ短くしましょう」とあります。』というコメントをいただきました。最近の言語でもそんな考えもあるんですね。ちょっと調べた限り、モジュール間で一貫して入れば短くても混乱しない、コードは短いほうが読みやすい等の理由があるそうです。自分が扱う段になったらもっと詳しく調べたいです。 ↩

  3. 『古いRDBMSやその後継システムの制約でテーブル名や項目名の長さに制限があり、その名残でアプリ側のコードも全体的にRDBMSに合わせざるを得なかったのかもしれません。』というコメントをいただきました。なるほどなるほど ↩

  4. 客先やコーディング規約で残さないと行けない環境もあるとご指摘をいただきました。本記事はgitやSVNがある環境を前提に書いております。 ↩

  5. C99より前のC言語では先頭で宣言する必要があったそうです。このあたりはコメントで@libraplanetさんが2021-02-22 09:02のコメントで詳しく書いてくださってます。この記事よりよっぽどわかりやすく有益ですので、ぜひ一読いただきたいです。 ↩

  6. コメントでいただいたものをそのまま、記載しております。ありがとうございます。 ↩

  7. 『万が一にでもテストから漏れると人が死ぬ可能性がある（自動車とか工場とか）性格のものは読みやすさはある程度犠牲にするみたいです．』というコメントをいただきました。たしかにそのような環境でプログラムをされている方であれば使用するのも納得できます。勉強になります。 ↩

  8. そう書いている自分も勉強不足で山程マサカリくらいました。勉強しろ！Sir, yes, sir! ↩

  9. Linqとか型推論とかは可読性・保守性に疑問があるので使わないというコメントをいただきましたが、それを判断できるレベルに私はありませんので、一般的に推奨されていることを書いています。 ↩

  10. CやC++では必要になるケースも有るとご指摘をいただきました。ここも@libraplanetさんの2021-02-22 10:48のコメントが詳しくてわかりやすいです。C#にも原始的なGOTOは存在するとの訂正も頂いております。私なんかよりよっぽど知識があって詳しいんだからわかりやすく記事にまとめてください！と思います笑 けど、そもそもいろんな知識があって周りに配慮できている人はこんな記事書かないか... ↩

  11. 『低レイヤーの領域でgotoの活躍の場があるため、C#にも仕様としてgotoが残されてるのかな、と思いました。』というコメントをいただきました。そういうケースもあるんですね。 ↩

448
374
75

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
448
374