Murga

個人的に言いたいコト・主張・気持ち。

読みづらいコードを見かけたから文句を言う

読みづらいコードを見かけたから文句を言う。

  1. 変数名が意味を表現していない。
    • 例 : itemNumberdataCount。前者はアイテムの ID かなんかかと思いきや個数 (length) を示すモノ、後者はループ中の index を示す変数名だった。
    • 何が悪いの? : 何のために存在する変数なのか、どんな影響があるのか、いちいち調べて覚えておかないといけなくなる。誤解も生まれやすい。
    • コメント : 名前重要。
  2. 意味に違いのない類語を混在させている。
    • 例 : insideinternal の表現が混在するが同じ意味合いで使っている・使い分けに意図がない。
    • 何が悪いの? : 読み手が表現の違いに理由があるのか調べて判断する手間が出てくる。変数名などで grep する時にモレやすい。
    • コメント : 名前重要。
  3. 全くコメントを書かない。書いてあってもクラス名と同じ文字列が書いてある見出し風のコメント行のみ。
    • 例 : かろうじて見つけたコメントが、設定用の XML ファイルに <!-- HOGE Parameters --> と書いてあるだけ。
    • 何が悪いの? : コードから「Why」が分からない。なぜこのような実装にしたのか、何が狙いかが分からない。すなわち、単に読みづらいコードなのか、何らかの理由でそうせざるを得なかったのか判断が付かず、汚いコードが余計にリファクタリングしづらくなる。
    • コメント : こういう場合は大抵 README も書いていないので、コーディングした本人にロングインタビューしないと解読できない。
  4. 概念的な共通点がないのに共通化する。
    • 例 : 人間とクルマ、両方「走れる」から Runner クラスで共通化だー!みたいな。まぁ英語でいえば rundrive で違うんだけど、それにしたって Runner クラスで共通化する意味が全くない。
    • 何が悪いの? : 本質的に (抽象概念的に) 共通する要素ではないので、子クラスごとに親クラスの実装内容を読み替えないといけなくなる。読み手の脳内バッファが食われて理解に時間がかかる。
    • コメント : 多分、目先のコードを共通化して使い回したいがためだけに、よく考えずにやらかしてる。「共通化」自体が正義だと思い込んでいる。
  5. 早すぎる共通化のせいで共通コードの中に種類別の分岐がある。
    • 例 : 先程の Runner クラスの中に、if 人間なら { … } else if クルマなら { … } みたいなコードがある。
    • 何が悪いの? : 汚いコードが不必要に散在して、リファクタリングが困難になっている。
    • コメント : この時点で「あれ?共通化できてないな?」と思えないということは、抽象概念として捉えられていない。
  6. コードの表面的にも、論理的な構造にも、規則性がない。
    • 例 : 空行の開け方に規則性がない。ディレクトリやファイルの分割単位に規則性がない。
    • 何が悪いの? : 「コレの場合はこう」「アレの場合はこう」と、色々覚えておきながらコードを読む必要があり、読み手の負担になる。
    • コメント : 「場当たり的」などというレベルではない不規則さ。わざと他の部分とは違う書き方をしようとしてるのだろうか。

書いた本人なら改修とかできるのかというと、本人も四苦八苦してる。もう捨てろよそんなコード、ってことで俺が全部書き直した。

「ざっくり」とか言うヤツ、いつまでも「きっちり」できない説

似たようなこと前にも書いたんですけどね…。

neos21.hatenablog.jp

  • ざっくりとですが叩き台を作りました」
  • 「時間ないからざっくり説明するね」

ざっくりしたモノは要らねえ、きっちりしたモン出せや

って思うんだけど、いつまでもきっちしりたモノは出てこない。

大抵は「ざっくりしたモノ」をベースに「ざっくりした別の何か」を作り、それが成果物と呼ばれ始めて、プロジェクト自体がざっくりと終わる。無駄に煩雑で分かりにくくなったメンテ手順やショボいバグの数々は全部運用フェーズに丸投げだ。

時間がないからざっくりしたモノしか作れない、という輩は、ほとんど 100% の確率で、潤沢な時間があったとしても、ざっくりとしたモノしか作れない。要はきっちりしたモノを作るスキルがないのだ。私は能無しだから誰か助けてくれって正直に言わんかい。


一番最初に用意するモノが「ざっくり」していると、それが最初に作られた以上の精度に精密化されることは、まずない。どうしてもそれが「基準」になってしまうからだ。

「本当はこのざっくりとした低品質な叩き台の全てが間違っていると思うんだけど、ゼロから全てを作り直すのは面倒臭いな、俺の責任になっても嫌だし」という、人間誰もが抱きうる逃げの気持ちが生まれるから、最初に作るモノがざっくりしていてはいけないのだ。

「曖昧な表現が飛び交う、ざっくりした要求仕様書」に基づいた開発プロジェクトが成功した試しがあるだろうか?初めに作る資料がざっくりしていなければ、何を作るべきか迷いはないし、万が一抜け漏れがあったとしても、きっちり書かれている中の抜け漏れなので、発見・対策が迅速に行いやすい。

叩き台を作るならざっくり作ってはいけない。きっちり作れ。


個人メモ、勉強した記録を「ざっくり」書いてしまうのは、整理がついていないためだと思う。

自分も、「とりあえずコレで動いたけど、原理を説明しきれないな…」というタイミングはあった。そんな時に書いたメモは「このオプションがないと動かないが何をしているかは分からない」とか、「とりあえずこの行があると上手く動く」なんて書いてあったりする。

でも、こんなメモをそのまま残しても、後で何の役にも立たない。書いてあるのを読み返しても、何も分かりゃしない。

自分の、誰かの役に立つメモにするなら、そんなざっくりした部分があってはいけない。

その日は理解が追いつかなくて曖昧なメモになってしまったら、それはそれで一旦残しておくのは良いが、後で必ず最初からやり直し、同じメモをゼロから書き直すことだ。

同じ内容を入力することになっても、前のメモからのコピペはせず、全てゼロから書き直す。コピペすると「ざっくりした品質」を移植することになるから、コピペは避ける。ゼロから検証をやり直し、メモを書き直すことで、前日よく分かっていなかった部分の理解も追いつき、前日よりはきっちりしたメモになっていることだろう。

きっちりしたモノを作るには、ざっくりしたモノをベースにしてはいけない。

きっちり! 恥ずかしくない! 文章が書ける

きっちり! 恥ずかしくない! 文章が書ける

省略時に適用されるデフォルト値を明示的にコーディングすべきか

記述を省略した時に、何らかのデフォルト値が適用されるプログラムに対して、

  • 「デフォルト値が自動設定されるなら、いちいち分かりきったことは書かなくていいべ」

と考えるか、

  • 「自動設定されるデフォルト値でろうと、その値が設定されて欲しいのであれば書くべき」

と考えるか。

例えば、何らかのデータを登録するための関数があったとする。

// ユーザ情報を保存する
hogeFugaApi.saveUser({
  name: 'Normal Man',
  age: 25
});

この saveUser() 関数のオプションには、ユーザの種類を特定する type プロパティも隠れていて、上のように type プロパティを指定しなかった場合は、自動的に type: 'member' が設定される仕様だったとする。もし 'member' 以外の種類で登録する場合は以下のように書くワケだ。

// 管理者ユーザ情報を保存する
hogeFugaApi.saveUser({
  name: 'Super Man',
  age: 30,
  type: 'admin'
});

今回問題にしたいのは、前者の Normal Man のような場合。type プロパティは省略可能だが、省略せず明記すると以下のようになる。

// ユーザ情報を保存する
hogeFugaApi.saveUser({
  name: 'Normal Man',
  age: 25,
  type: 'member'  // ← 記載を省略した時のデフォルト値と同じ
});

省略すれば type: 'member' と書いたのと同じ状態になるワケだから、書かなくても良い、と考えるか。それとも、type: 'member' を律儀に書くべき、と考えるか。というのが今回の話。

「省略すれば良い」派の考えを想像する

まずは「省略すれば良い」と思う人の考えを想像してみる。

  • 省略時に適用される値をわざわざ書くのは、コード量も増えるし煩雑になるのでは
  • そのオプションを省略した時に、何のデフォルト値が設定されるかは、リファレンスなどに書いてあるだろうから、分かりきったことは書かない
  • デフォルト値になっていれb良かったところも、いちいち記述したせいで、書き間違いによってバグを埋め込んだりするリスクもあるだろう

「省略せず書くべき」派の考えを想像する

次は反対意見を想像してみる。

  • デフォルト値であろうと、その値が設定されるべきなのであれば、多少冗長であっても明示的に指定した方が分かりやすい
  • コードの読み手は、省略した時に何がデフォルト値になるのか分からないかもしれない
  • 呼び出す関数 (ライブラリなど) 側の仕様変更によって、省略時のデフォルト値が変わるとも限らないから、明示した方が安全

双方のシチュエーションが若干異なる

両者の意見を想像して、じゃあどちらがより良いのだろうか〜、と考え始めたが、よくよく見直すと両者が想像するシチュエーションには若干の違いがあることに気付いた。

「省略して良い」派は、「単に書くのが面倒臭い」「書かなくても分かるっしょ」という利己的な思想も強いが、「デフォルト値だと思ってわざわざ書いた値が間違っていた場合はどうする?」という点を掘り下げていくと、「デフォルト値は何でも良い場合」があると思った。

すなわち、最初に書いたサンプルコードの場合、typemember になろうが admin になろうがどうでも良い場合、というモノがあれば、記載を省略して何らかのデフォルト値を適用させてしまって良い、ということだ。

一方、「省略せず書くべき」派は、「明示することで読み手が分かりやすくなる」「呼び出し先の仕様変更も考慮して安全にしたい」という配慮が根本にあるが、コチラの場合は「確実に指定したい値が、たまたまデフォルト値と同じだっただけ」という場合が多い気がする。

すなわち、元々 type: 'member' を設定しないといけない、と思ってコーディングしていたが、それがたまたま対象の関数のデフォルト値と一致していただけ、という考え方だ。この場合は「何でも良いデフォルト値を指定したい」のではなく、「type: 'member' を指定したい」のだから、明示的に書きたくなる気持ちも分かるだろう。

どちらが「良いコード」?

それぞれの立場が論じるシチュエーションが若干異なるので、

  • 省略時に適用されるデフォルト値が何の値であろうと構わないなら、省略しても問題ない
  • その値を指定することに何らかの狙いがある (そしてそれがたまたまデフォルト値と同じだけ) なら、省略せず書いた方が安全

と判断しても良いだろう。

ただ、ほとんどの場合において、「省略時に適用される値が何でも良い」というパターンは発生しないのではないだろうか。例えば「性別未選択の場合」に、

  • null とするのか
  • 空文字とするのか
  • どの性別コードとも合致しない -10 などを入れるのか
  • '未選択' と文字列で入れるのか

これが「どの値でも良い」としているシステムって、なかなかないと思う。

そう思うと、「記載を省略した方が良い場合」は限りなく少なく迷うくらいなら明記した方が良いと思う。

プログラムはよく「書いたとおりに動く」と言われる。「書いたとおりに動いた結果がバグっている」なら、書いてある内容を正せば直せる。しかし、「何も書いていない場所がバグっている」のは、バグの原因究明がより難しいと思う。そういう意味でも、書き間違いなどのリスクよりも、確実に書いたことによる「安全」「後の楽さ」を取った方が良いと思う。

リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice)

リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice)

  • 作者: Dustin Boswell,Trevor Foucher,須藤功平,角征典
  • 出版社/メーカー: オライリージャパン
  • 発売日: 2012/06/23
  • メディア: 単行本(ソフトカバー)
  • 購入: 68人 クリック: 1,802回
  • この商品を含むブログ (140件) を見る