「それ、もっとスマートに書けるよ」批判 | blog.tai2.net

昨年末に、それ、もっとスマートに書けるよ - JavaScript コードをもっと短く、もっとシンプルに書く Tips 4選というスライドがツイッターなどで話題に登りました。

JavaScriptにおいてスマートなコードを書くための、4つのTipsを紹介するという内容です。スライドでは、まず「改善前」のコードが示され、それらをどのように「改善」できるかが示されます。しかし、4つのうち2つの例は、改善されているとは思えないものでした。元のコードのほうが良く見えるのです。¹ 他の人々の反応を見ても、同様の否定的な見解が多いようですが、中には感銘を受けているようなコメントもいくつか見られました。

この記事では、なぜそれらのコードが良くないのかと考えることで、チームにおけるコーディングはどのようにあるべきか、という問いを追求します。

それでは、2つのTipsを見ていきましょう。

1. Arrya.prototype.indexOf()

最初のTipは、ユーザーエージェントの判定を例にしています。元のコードはこちら。²

const ua = navigator.userAgent;

if (ua.indexOf('iPhone') > -1 || ua.indexOf('iPod') > -1 || ua.indexOf('iPad') > -1) {
    return 'ios';
} else {
    return 'other';
}

これを「スマート」にすると、このようになります。 (以後、本記事では、発表資料で挙げられている「改善」後のコードを「スマート」なコードと呼ぶことにします。)

const ua = navigator.userAgent;

if (~ua.indexOf('iPhone') || ~ua.indexOf('iPod') || ~ua.indexOf('iPad')) {
    return 'ios';
} else {
    return 'other';
}

2の補数表現において、-1はすべてのビットが立ちます。ビット反転演算子を使うことで、-1は、すべてのビットがゼロで表現される数、すなわち0になるという事実を利用したトリックです。

ちなみに、発表者のwakamshaさんは、「整数に対して実行すると符号が反転して-1した値となる」と発言していることから、整数にチルダ演算子を適用することの意味を理解せずに、このトリックを使っているようです。-1以外の負数に対して試してみればわかりますが、単純に符号反転するわけではありません。

2. let / const

次の例は、 const 変数を初期化する値を分岐させるというものです。

元のコードはこちら。

let foo;
let bar;

if (new Date().getHours() < 12) {
    foo = 'forenoon';
    bar = 'am';
} else {
    foo = 'afternoon';
    bar = 'pm';
}

「スマート」にするとこうなります。

const {foo, bar} = (() => {
    if (new Date().getHours() < 12) {
        return {
            foo: 'forenoon',
            bar: 'am'
        }
    } else {
        return {
            foo: 'afternoon',
            bar: 'pm'
        }
    }
})();

即時関数(IIFE)を使うことで、分岐を式として扱うトリックです。

最初のif文を使って書く方法は読み易いし、これで十分だと思います。どうしても const で済ませたければ、このように三項演算子で書けます。

const { foo, bar } = new Date().getHours() < 12
    ? { foo: 'forenoon',  bar: 'am' }
    : { foo: 'afternoon', bar: 'pm' }

分岐内に複数のステートメントを書きたい場合は、やはり let と if でいいと思います。あるいは、即時関数ではなく、ちゃんと名前をつけて別の関数にしてそれを呼び出せば素直なコードになります。

チームにおけるコードはどうあるべきか

チーム開発におけるコードは、リーダブルであるべきです。それも、ただ読み辛くなければいいというだけではなく、全力で読み易さを追求するというのが、あるべき姿であると筆者は考えます。例え、局所的に、ほんの少し実行効率や空間効率が悪くなったとしても、リーダブルなコードを書くことのほうが大事です。

なぜリーダブルなコードが大事なのでしょうか。

実際の開発におけるコードというのは、基本的に書く回数よりも読む回数のほうが多いものだから、というのがひとつの理由です。一度書かれたコードは、開発に参加する何人もの人に読まれますし、自分自身も何度も読み返します。半年後の自分は他人である、というのはよく言われることです。ですから、なるべく読んですぐに理解できるコード、読解するために考えたり調べたりして、ストレスを感じることのないコードが理想です。

リーダブルなコードはどういうものか

ところで、リーダブルなコードとはどういったものでしょうか。

おそらく、wakamshaさんにとっては、「スマート」なコードは、十分に読めるコードなのだと思います。

先日の勉強会資料が急にはてブホッテントリ入りしてたので何事かと思ったら、随分とネガティブなコメントが集まってて「まぁ…」と驚いてます。

トリッキーなのは認識してますがそんなに解りづらいかな？
— wakamsha / Naoki.YAMADA (@wakamsha) October 8, 2017

人によって、どういうコードを読み易い、あるいは読み辛いと感じるかは、千差万別です。万人に通じる絶対的に読み易いコードというのはないのかもしれません。

読み易さにとって重要な基準となるのが、それがふつうの書き方であるかどうかということです。なにをもって「ふつうでない」とするかも、個別に議論し出すと難しくなってはくるのですが、基本的には、いろいろな人のコードを読んで経験を詰めば感覚としてわかってくると思います。

「スマート」なコードとして書かれている上記の2例は、どちらもふつうの書き方ではないと思います。いままであのような書き方をしているコードを見たことがありません。ですから、ひどく違和感を覚えるのです。

もちろん、それを補ってあまりある合理的なメリットがあるのであれば、ふつうでないコードを書いても良いと思います(ただし、その場合には、なぜふつうでない書き方をしているのかをコメントに書いておく必要があります)。「スマート」なコードに、ふつうさを犠牲にする合理性があるでしょうか。そうは思えません。

ふつうさと並んで読み易さに関係してくるのが、その機能が持っている本来の意図と、使い方が一致しているか、ということです。チルダ演算子は、オペランドが-1かどうかを判定するための演算子ではないですし、IIFEは、ES5以前にレキシカルスコープを模倣するために発明されたテクニックです。本来の目的と使い方が一致していないために読むものに驚きを与えてしまっているという面はあると思います。

本来の機能と使用法の一致に関しては、但し書きが必要です。たとえば、 JavaScriptのASI(自動セミコロン挿入) は、元々はセミコロンを書き忘れても動くことを意図した機能ですが、今では、JavaScriptをセミコロン不要な言語とするためにASIを積極的に利用することが、コミュニティーにおいてある程度の市民権を得ています。

本来の機能と使用法が一致しているべきというのは、絶対的な規則ではありません。多くの人に受け入れられているのであれば、たとえ機能の本来の意図と一致していなくても許されます。つまり、ふつうのコードであるということです。もしもチルダ演算子で-1かどうかを判定するテクニックが多くの人に受け入れられ使われている書き方であったなら、筆者もそれを受け入れます。ただし、相応の合理的なメリットがなければ、多くの人に受け入れられることはないでしょう。

問題点

以上の議論をふまえて、紹介されている2つのテクニックはどこが良くなかったのでしょうか。

まず両方に共通して言えることとして、どちらもふつうの書き方ではありません。ふつうの書き方だったら、わざわざ勉強会で発表したりはしないと思いますし。また、ふつうでない書き方をするに足る合理的なメリットもないと筆者は考えます。

1のコードを目にしたプログラマーの多くは、なぜ唐突にビット反転演算子が使われているのか、なにか特別な意味があるのか、また、ビットを反転することによって実行結果がどうなるのか、頭を抱えることでしょう。

2については、すこし補足が必要かもしれません。このテクニックは、たしかに、 let を排除するという機能的なメリットを提供しています。基本的に const を使うという方針は正しいです(文字数が長くなるくらいしかデメリットがないので)。かといって、それが絶対的に厳守しなければならないルールかというと、そうではないと思います。

const を厳守するために無理な書き方をするくらいなら、筆者は let を使います。 let ならば var と違ってスコープも宣言以降に限定されていますし、スコープがよほど長くならなければ(100行以上に跨るとかだと厳しいかもしれません)、目視で十分に確認できます。問題ありません。

IIFEはそもそも読み易い書き方ではありません。トリッキーな書き方ではありますが、それを補うほどにグローバルスコープを排除するということは重要でした。ですから、モジュールの概念もブロックスコープ変数もなかった時代に、他にやりかたがないのでしかたがなく使っていたのです。たかだか関数内での let を排除するという小さな目的のために持ち出すようなものではないと思います。

なにが良いコードで、なにが良くないのかについて合意を形成するのは、各人が持っている経験やバックグラウンドが異なる実際の開発において、相当困難であるということは、筆者も身を持ってわかっているつもりです。ですから、コードの書き方については、ある程度の個性を許容するということは、チーム作業においてどうしても必要になってくると思います。

それでも、上記のような「スマート」なコードは、できることなら書かないで欲しいと願います。

まとめ

愚直なコードを書くのは悪いことではありません。小手先の「スマート」な書き方にほんとうにメリットがあるのか、立ち止まって考えてみましょう。

2番目の console.log はデバッグ時の一時的な処置なので問題なし。4番目の reduce に直す例は、よくあるふつうの書き方で、改善されていると思います ↩
indexOf で、対象が含まれないかどうかの判定は、筆者なら > ではなく != を使います。存在しないことを表す特殊な値「ではない」ことを確認する、という意図を明確に表現したいからです。ここでやりたいのは、数値の大小比較ではありません。 ↩