昔の自分を反面教師として学ぶ JavaScript

ふと昔自分が書いたコードを眺めてました。
あるあるだと思うんですが、「ひどいなー」とか「今ならこうするなー」とか思ったりするわけです。

今日はそんなひどいコードたちを、自分への戒めという意味も込めて、正していこうと思います。どれも基本的なことばかりですが。

ちなみに、ここで挙げている昔のコードは、いずれもバグ予備軍的なコードだったり、ちょっとイケてなかったりするコードです。
そのコードが動く環境では期待通り動いてます。

本当にバグを見つけちゃったら、こんな感じでは書けませんね。

for 文の継続条件式におけるプロパティ参照

昔のコード:

for (var i = 0; i < items.length; i++) {
  // ...
}

今ならこう書く:

for (var i = 0, length = items.length; i < length; i++) {
  // ...
}

JavaScript に限らず基本ですね。
ループの度に items.length が評価されちゃうのでパフォーマンス的に不利です。

初期化式で length を定義して items.length への参照が一度で済むようにします。

undefined の判定

昔のコード:

obj === undefined

今ならこう書く:

// 1
(function(undefined) {
  obj === undefined;
})();

// 2
typeof obj == 'undefined'

// 3
obj === void 0

昔のコードでも、多くの場合、期待通り動きます。

ただ、undefined は予約語ではなく、グローバルオブジェクト (ブラウザなら window) のプロパティなので、他の値で書き換え可能です。
（最近はできなくなりつつある？）

ですので、素直に undefined と比較するのではなく、以下の方法をとるのがベターです。

1. 引数の省略を利用して確実に undefined を取得する
2. typeof 演算子を使う
3. void 演算子を使う

null か undefined の判定

昔のコード:

typeof obj === 'undefined' || obj === null

今ならこう書く:

obj == null

ガード節としてよく用いるものですね。昔のコードでも問題はないと思います。

ただ、obj == null で null か undefined であるかを判定出来るので、今はそちらを使うようにしてます。
「== は使うな」的な風潮がある気がしますが、null との比較はイディオム的に使っていいんじゃないかなーと思います。

Array の判定

昔のコード:

obj instanceof Array

今ならこう書く:

Object.prototype.toString.call(obj) === '[object Array]'

挙動をきちんと理解した上で使うのであれば、昔のコードでも問題ないと思います。

両者の違いとして、instanceof はプロトタイプチェーンを辿るので以下のような場合でも true となってしまいます。

var Foo = function() {};
Foo.prototype = [];
var foo = new Foo();

console.log(foo instanceof Array); //=> true

Object.prototype.toString を利用すれば、組み込みの Array オブジェクトの場合のみ true となります。
これは「今ならこう書く」っていうか使い分けでしょうか。

ラッパーオブジェクト

昔のコード:

var s = new String('foo');

今ならこう書く:

var s = 'foo';

JavaScript にはプリミティブな型と、それに対応するラッパーオブジェクトが存在します。(Number, Boolean, String)
昔のコードではラッパーオブジェクト、今のコードではプリミティブ型を利用しています。

それぞれどう違うのかは割愛しますが、JavaScript において明示的にラッパーオブジェクトが必要な場面ってのは、ほとんど無い気がします。
「typeof で 'object' が返ってきて・・・」とか、いらぬバグを生まない為にも、極力プリミティブ型で統一するようにしましょう。

多分当時は「new を付けた方がオブジェクト指向的でかっこいい」って感じで、意味も分からず使ってたんだと思います。

Array コンストラクタ

昔のコード:

var items = new Array();

今ならこう書く:

var items = [];

いずれも空の Array を生成してます。
Array コンストラクタを使うか、配列リテラルを使うかの問題ですね。

Array コンストラクタは引数が一つで整数の場合、引数の数だけ undefined で初期化した Array が生成されます。
一方、配列リテラルの場合、常にリテラルに渡した値で Array が生成されます。

new Array('foo', 'bar', 'baz'); //=> ['foo', 'bar', 'baz']
['foo', 'bar', 'baz'];          //=> ['foo', 'bar', 'baz']

new Array(3); //=> [undefined, undefined, undefined]
[3];          //=> [3]

思わぬバグを生まない為にも、今は一律 Array リテラルを使うようにしてます。

arguments の扱い

昔のコード:

function foo() {
  var args = [];
  for (var i = 0; i < arguments.length; i++) {
    args.push(arguments[i]);
  }
  // ...
}

今ならこう書く:

function foo() {
  var args = Array.prototype.slice.call(arguments);
  // ...
}

関数の実引数が格納される arguments は「Array のようなオブジェクト」であって Array ではありません。
当時もそのことは知っていたようですが、それを Array に変換するコードが冗長です。

Array.prototype.slice を使えば、「Array のようなオブジェクト」を本物の Array に変換することができます。
同メソッドは意図して汎用的に作られていて、「Array のようなオブジェクト (length を持っててインデックスで要素にアクセスできる)」に対しても呼び出せるようになってます。

Arguments の他にも HTMLCollection や NodeList なんかも変換可能です。

グローバル汚染

昔のコード:

// ここはトップレベル

var foo = 'foo';

function hoge() {
  // ...
}

// ...

今ならこう書く:

(function() {
  var foo = 'foo';

  function hoge() {
    // ...
  }

  // ...
})();

ダメ。ゼッタイ。
name や parent など、案外あっさりと window のプロパティ競合しちゃったりするものです。

即時関数を利用してスコープを作りましょう。

プロトタイプ汚染

昔のコード:

Object.prototype.clone = function() {
  // ...
};

今ならこう書く:

function clone(obj) {
  // ...
}

JavaScript ではプロトタイプを拡張することで、組み込みのオブジェクトにメソッドやプロパティを追加できます。

でも、for in で列挙されるようになったりとか、同名のメソッドの間で実装に差異が出たりとか、そこにはいろいろ弊害があります。
もしかしたら ECMAScript 標準で同名のメソッドが定義される可能性もあるわけです。

拡張したいオブジェクトを引数とする、underscore.js 的なアプローチを取りましょう。

Object にメソッドが追加できると知って、ドヤ顔で書いていた当時の自分が目に浮かびます・・・。

その他のひどいコード

Array の判定？

if ('push' in obj && 'pop' in obj) {
  for (var i = 0; i < obj.length; i++) {
    // ...
  }
}

push, pop っていうプロパティを持ってるかどうかで配列であるかを判定しています。
ECMAScript 標準のオブジェクトに限れば、一応これでも判定できます。多分。

このコードのだめなところは、push, pop っていうインターフェースを期待しておきながら、
スタック的な使い方を一切せず、普通に length を使って for ループしてることです。

ダックタイピングの真似事だったんだと思うんですが、全然出来てませんね。

エイリアス？

// ここはトップレベル

var Foo = {
  hoge: function() {
    // ...
  },
  fuga: this.hoge
}

これは本当にひどい。

Foo.fuga を Foo.hoge のエイリアスにしたかったんだと思います。
でもこのコードだと this.hoge は window.hoge を参照してしまいますので、意図した挙動はとりません。

この例でいう fuga を使ってなかったようなので、エラーにはならかったようですが・・・使わないならこんな事しちゃいけませんね。

当時の自分の意図を汲んでみると、こんな感じでしょうか。

var Foo = {
  hoge: function() {
    // ...
  }
}
Foo.fuga = Foo.hoge;

振り返って

図らずも雑多な JavaScript Tips みたいになっちゃいました。

JavaScript って、こういうちょっとした落とし穴みたいなのが本当に多いですよね。
なぜ Coffee Script 等が話題になってるのかがよくわかります。

でも僕は好きですよ。JavaScript のこういうところ。