こんなコードは嫌だ、おじいコード駆逐したい(とりあえず9つ)

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

転職してきた若いプログラマが変なコード書いている。
どうやら前社の社内研修で教わったとのこと。
さて、老害は何を教えたのだろうか。
※一応TypeScriptで書きましたが別にC#とかPythonでも言えることです。

1.変数名が雑

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

10行程度の短い関数ならretでもdataとか適当な名前でもいいけど
長くなるようならちゃんと名前つけてください。

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

public Piyo() : void {
    data = this.getData();
    // 十行以上のコード

    // このデータなんやねん!ちゃんと名前ついてないからわからん!
    if(data.result){
    }
}

2.無駄な省略

これも命名なんですけどね。
古い習慣なんですかね。
意味わからんからやめてね。
countをcntとか、managerをmgrとか、meetingをmtgとか
日常のメールとかに書く分には好きにしたらええけどプログラムには書かないでね。
特に関数内のちょっとした変数ならともかく、
クラス名になんてしないでね。

public Hoge() : void {
    // は?dtってなんやねん、DateTimeか?童○か?
    let dt = this.getData();
    // 以降なんかの処理
}

3.無駄に()つけている

if文で無駄な()が多いと読みにくいです。
C++とかではそんな書き方推奨なんですか?

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

可読性を気にして、そんな書き方するぐらいなら
一回変数で受けたほうが読みやすいです。
※上記の例程度なら変数で受ける必要もないので、無駄な()だけ消してね。

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

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

これは、そのまんまですね。
コード管理にgitなりSVNなり使っている職場でコメントアウトして
残すとかやらないでね。
gitもSVNもなく、差分管理してないような職場なら転職考えてね。

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

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

変数の宣言する場所考えてますか?
CとかC++とかは関数の最初に宣言するんだっけ?知らん。
とりあえず必要最小限のスコープで宣言してね。

public getPrice(fuga : boolean, piyo : boolean) : number {
    let price = 0;
    if(fuga){
        price += 100;
        // ここでitem宣言する必要ある〜?
        const item = this.getItem();
        if (piyo){
            // このif文の中でしかitem使ってないよね?この中で宣言しようね?
            price += item.Price;
        }
    }
    return price;
}

6.ハンガリアン

変数の宣言時に変数の頭に型がわかるように書くやつです。
個人的にはそこまで嫌いじゃないけど令和の時代に書いてたら笑われるで。

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

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

7.ヨーダ記法

定数をif文の左側にかくやつです。
読みにくいのでやめてね。
ヨーダ記法 - Wikipedia

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

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

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

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

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

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

// 推奨パターン
public goodHasNumber(targetNumber : number){
    // 一行で済む
    return _numbers.some(n => n == targetNumber);
}

9.goto

いや、ほんま何を教えとるんですか。
教えた人は引退して、どうぞ。
TypeScriptではないんやね?
とりあえず、どのgoto必須の言語以外でgoto使おうなんて考えないでね。
C#で使ってたりしたら、ふざけてるとしか思わないからね。

最後に

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

yuu_j
経済学部⇒PHPer⇒C#er Pythonで機械学習とSwiftでi Phoneアプリ作りたい
ユーザー登録して、Qiitaをもっと便利に使ってみませんか。
  1. あなたにマッチした記事をお届けします
    ユーザーやタグをフォローすることで、あなたが興味を持つ技術分野の情報をまとめてキャッチアップできます
  2. 便利な情報をあとで効率的に読み返せます
    気に入った記事を「ストック」することで、あとからすぐに検索できます
この記事は以下の記事からリンクされています
過去の1件を表示する
コメント

3.無駄に()つけている

if文で無駄な()が多いと読みにくいです。
C++とかではそんな書き方推奨なんですか?

演算子の優先順位で自明なところへの無駄なカッコの使用を勧めてくれる C++コンパイラはありますね。

void piyo(bool a, bool b, bool c)
{
    if (a || b && c) hoge();
}
prog.cc: In function 'void piyo(bool, bool, bool)':
prog.cc:4:16: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
    4 |     if (a || b && c) hoge();
      |              ~~^~~~

https://wandbox.org/permlink/ZjsWEv2G1JJDkuTC

自分もこの場合無駄でもカッコを使用したほうが良いと思います。

void piyo(bool a, bool b, bool c)
{
    if (a || (b && c)) hoge();
}

TypeScript界隈では老害扱いされる書き方なんでしょうか?

@fujitanozomu
ご指摘いただきありがとうございます。
TypeScript界隈のことは存じておりませんが、
個人的には、そのあたりは好みの問題だと思います。
or条件とand条件を併用する必要がある場合は私もそのようにカッコをつけます。

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

個人的には↑のカッコも悪いと思わないのですが

個人的には、そのあたりは好みの問題だと思います。

カッコの有無がルールを明文化できないところでの個人の嗜好で良し悪しを論じている程度のものであるならば自由で良いのでは? 変な縛りを設けたところで若い人を委縮させるだけじゃないかと思いますね。

(編集済み)

@fujitanozomu
そのあたりはC++文化で昔はそうだったんだなぁといった感想です。
C#の経歴が主ですが、VisualStudioを使って開発しており
そのような無駄なカッコを見かけることはありませんでした。

でも実際どうなんでしょうね?
新しめの言語、VS Codeとか多機能なIDEを使っている環境でも
無駄なカッコあった方がいいんですかね?

個人的にはない方がいいと思っているので書かせていただきました。
あった方がいいとは思わないので。

若い人を萎縮させるかどうかはコミュニケーション能力の問題だと思います。

追記:
私もTypeScript、JavaScriptは勉強中なのですが、
下記のサイトの例を見る限り
@fujitanozomu さんがはじめに挙げられているorとandが混在する例ではカッコをつけろって
ってなってます。それ以外の例ではつけてないですね。
https://github.com/airbnb/javascript
下記の記事から上記サイトを参考にさせていただきました。勉強になります。
Webエンジニアが勉強できるGit Repository 10選

C#の経歴が主ですが、VisualStudioを使って開発しており
そのような無駄なカッコを見かけることはありませんでした。

さぞや経験を積まれてる方なのだろうと思いますが自分が見たことがない ⇒ 悪という判断は危険ではないでしょうか

https://docs.microsoft.com/ja-jp/dotnet/csharp/programming-guide/inside-a-program/coding-conventions#layout-conventions

  • Use parentheses to make clauses in an expression apparent, as shown in the following code.
C#
if ((val1 > val2) && (val1 > val3))
{
    // Take appropriate action.
}

あと、

orとandが混在する例ではカッコをつけろって
ってなってます。それ以外の例ではつけてないですね

挙げられるべきは無駄なカッコが悪い書き方であるという記述では?

ヨーダ記法と無駄な括弧に関しては組み込み系で使われているイメージがありますね.
万が一にでもテストから漏れると人が死ぬ可能性がある(自動車とか工場とか)性格のものは読みやすさはある程度犠牲にするみたいです.

指摘してる人もいますが、その記法に意図なり意味があるのを全否定されてもね
うちの職場で使わないってだけの話を一般論にされても疑問しかないですね

「藪をつついて蛇を出す」を地でいく事例ですね。プロジェクトの規約でこうしろと定められている場合とか、部署のコーディング規約とか、チーム内の取り決めとか、現実に直面する

「個人ではどうしようもない制約」

が存在することを知っていれば、同じ指摘でも今回のような表現にはならなかったはずだと思います。

>「さぞや経験を積まれている方」

という突っ込みがありましたが、これはいわゆる「慇懃無礼」な物言いです。この一言を歯に衣着せずに表現するなら

「なに指導者ぶってんだよ素人が」

になるでしょうね。己の力量とか業界の悪しき風習について理解を深めておくことも技術者として大切なことですよ。

コーディングルールはeslintで指摘するべきで、人間が見ているのは前時代的では?

私 ハンガリアン、めっちゃ使ってます :grinning:
Visual Basic から入ったからかな?

むかし、ハンガリアンを拒否したい人が、
「接頭語の n や s の1字入力が積もり積もれば無駄な時間が増える」
と謎の反論してた :dizzy_face:

個人的には悪い例を「おじいコード」と呼んで侮辱的に表現している点がもっとも罪深いと考えています。その思想は記事のすみずみに現れており、高所から見下すような表現が多く見られますね。

コードの良否と書いた人間への感情的な評価を同一にしてはいけないでしょう。

それはハラスメントにつながります。

(編集済み)

自分は仕事のスタイルとして色々な会社を大体半年~1年単位で転々としているのですが、ここで語られているような事が仕事場ごとに規約の微妙な異差として常に付きまとい、それに関して(クセとして付いた部分を次の職場に合わせる)対応を迫られるわけでして

それ自体は問題ないのですが、ただ移る度にそこの職場の人間がさも自分の所で取り入れている規約を全ての人に通じるこの世の唯一真理のように語り、それを守れていない人に対してまるで人の道を外れている人を見るかのような胡散臭げな目で見るのに嫌気がさしている次第です

ここで語られていること、例えば、単語を略すかどうかに関して、「一般的な単語なら略してもいいよ」としている所もあれば、どの単語に限って略せるかを明確に規約に盛り込んでいるところもあれば、「とにかく絶対に略すな」という事を規約に明確に取り込んだ結果、グローバルの変数名や関数名が長くて100文字を越えるような状況になっていてみんなうんざり、みたいな職場もあります

また、職場によって「1つの関数が○○行を越える(要は1画面に収まらない)なら、分離する」ような事を盛り込んでいるような規約もあり、それが徹底している職場ならローカルの変数名を略す事に懸念されるような害はさほど発生しないのではないかと思えます(むしろより見やすくなるかもしれません)

詰まるところ、どちらがいいということではなくどちらがその現場に適しているかという話になるわけで、ならば本質的な所として「(現場として略すべきではないのなら)それが規約に記載されているか」「(言わなくても常識だろ、ではなく)それを守らせるための周知教育が徹底しているか」という所を考えるべきように思えます

あなたもコメントしてみませんか :)
すでにアカウントを持っている方は
ユーザーは見つかりませんでした