あるドロップダウンボックスが表示されているときに、別のドロップダウンボックスを制御できるコードが、SitePointフォーラムの最近のスレッドで話題になっています。そのコードは確かに動作していましたが、実は不十分であることが分かりました。脆弱で、HTMLのちょっとした変化に耐えられないものだったのです。
元のCSSコードは次のとおりです。
#second { display: none; }
#second.show { display: block; }
そして元のJavaScriptコードは次のとおりです。
document.getElementById("location").onchange = function () {
if (this[this.selectedIndex].value === "loc5") {
document.getElementById("second").className = "show";
} else {
document.getElementById("second").className = "";
}
};
本記事では、再利用を容易にし、その後の変化に対してもっと柔軟に対応できる、JavaScriptの簡単なリファクタリング手法を説明します。
どの方法にするか見分けるには
JavaScriptには同じタスクを達成する方法がいろいろとあり、そのうちのいくつかはほかの方法よりもうまく動作します。手戻りが必要ないように、いますぐコードを改善する方法はあるのでしょうか。もちろんあります！ でも有力な方法がいくつかある場合、最適な方法をどのようにして決めたらよいのでしょう？
コードを改善する一般的な手法の1つは、（DRYの原則を利用して）重複を取り除くことです。そのことが、特定のコードをもっと幅広い状況に対処できる汎用的なコードに変更するのに役立ちます。
汎用的ではないコードはわずかな変更で不安定になる傾向があります。コードは真空空間にあるのではなく、コードの周辺やHTMLコード内のほかのアクションに応じて変更する必要があります。過去の経験から、一般的に起こる変化を調べ、重複を減らすことで改善ができます。
でも、注意してください！ 理解が難しくなるまで汎用的にしすぎてしまうことがあります。汎用的であることと、読み取り可能であることの間でバランスの良い状態にすることが、「コードを改善する」ということです。
JavaScriptのリファクタリング手法：汎用対象
テスト駆動開発（TDD）の過程では、プロセスの一環として、この原則との遭遇は避けられません。
テストはより具体的になり、コードはより一般的なものになる。
Robert C. MartinによるTDDのサイクルは、この考えをうまくカバーしています。主なメリットは、汎用コードが最後には幅広い状況やシナリオを扱えるようになることです。
前のコードを見ると、いくつかの一般的な改善策が明らかにすぐに使用できます。
- 変数に文字列を格納すると、1つの場所で文字列を管理するのに便利
- onchangeイベントハンドラは、上書きされる可能性があり問題なので、代わりに、addEventListenerの使用を検討する
- classNameプロパティは既存のクラス名を上書きするので、代わりに、classListの使用を検討する
すべて改善すれば、将来の変化に対して柔軟性があり、更新が簡単なコードになります。それでは始めます。
重複を防ぐために変数を使う
ドロップダウンボックス（location）のIDとそのトリガ値（loc5）は、ひとまとめにして参照するのに適しています。また、2番目の<select>要素が2度参照されていますが、混乱を防止しメンテナンスをより簡単にするために別々の変数へ取り出せます。
たとえば、要素のIDが変更された場合、同じ要素を参照している２箇所を変更しなければなりません。
// bad code
if (...) {
document.getElementById("second").className = "show";
} else {
document.getElementById("second").className = "";
}
要素への参照を変数に格納することで、今後変更する場所を変数が割り当てられている場所にのみ制限できます。
// good code
var target = document.getElementById("second");
if (...) {
target.className = "show";
} else {
target.className = "";
}
これらの文字列をコードの先頭へ取り出し、if文の一部を分岐することで、コードを汎用化し管理が楽なコードにできます。識別子またはオプション値のいずれかが変更された場合、コードの隅々を探しまわる代わりに、1つの場所ですべて簡単に見つけられます。
// improved code
var source = document.getElementById("location");
var target = document.getElementById("second");
var triggerValue = "loc5";
source.onchange = function () {
var selectedValue = this[this.selectedIndex].value;
if (selectedValue === triggerValue) {
target.className = "show";
} else {
target.className = "";
}
};
イベント処理の改善
昔から使われているイベントハンドラは、いまでもとてもよく使われています（そしてこのケースでは正しく使用されています）が、いくつか問題があります。もっとも大きな問題は、昔からの方法で要素のイベントハンドラを設定すると、同じイベントの以前のハンドラを上書きしてしまうことです。
// bad code
source.onchange = function () {
// ...
};
現在、上のコードは動作します。テストを使って実証できます。
テストについての簡単なメモ
理念：テストは書いたコードが期待どおりに動作しているか確認する素晴らしい方法です。コードの変更が、別の所にあるなにかを破損する原因となる可能性を減らします。テストの導入については、残念ながら本記事の範囲外です（ただし、SitePointには、このトピックに関するたくさんの素晴らしいコンテンツがあります）。しかし、多くの人はまったくテストを記述せずに理解できます。
構文：次のテストは、Jasmineのテストフレームワークを使用しています。Jasmineのテスト（別名スペック）は、Jasmineのグローバル関数itを呼び出して定義され、文字列およびさらなる関数を引数として受け取ります。文字列はスペックのタイトルで、関数はスペックそのものです。Jasmineについてはプロジェクトのホームページで詳細を確認できます。
テストを実行する
コードの以前の状態を前提とすると、次の2つのテストをクリアできます。
it("should add the 'show' class name when the 'loc5' option is selected", function() {
changeSelectTo(source, "loc5");
expect(target.classList.contains("show")).toBe(true);
});
it("should remove the 'show' class name when an option value different from 'loc5' is selected", function() {
changeSelectTo(source, "loc2");
expect(target.classList.contains("show")).toBe(false);
});
changeSelectTo関数は、要素が正しいクラス名を持っている<select>要素の値と（Jasmineのexpect関数を使って構築された）期待値を変更します。
ただし、onchangeハンドラが変更される（ほかのコードでも変更できてしまう）とすぐに、クラス名が変更された関数が失われ、うまく動作しなくなります。これは追加のテストで実証できます。
it("should toggle the class name even when the onchange event is replaced", function () {
changeSelectTo(source, "loc2");
expect(target.classList.contains("show")).toBe(false);
// Overwrite the onchange handler
source.onchange = function doNothing() { return; };
changeSelectTo(source, "loc5");
expect(target.classList.contains("show")).toBe(true); // fails
});
このテストは、CodePenで示すように失敗します。Jasmine固有のコードは別のPenにあることに注意してください。
テストにパスするためにコードをリファクタリングする
1つのイベントに任意の数の機能を割り当てられるようにするaddEventListenerを使用すれば、簡単に先のテストをパスできます。falseパラメーターは、イベントの順序にイベントキャプチャー（trueのとき）か、イベントバブリング（falseのとき）かが使用されていることを宣言します。Quirksモードは、イベントのイベント順序の、優れた概要を示します。
// good code
source.addEventListener("change", function (evt) {
// ...
}, false);
下のコードは、この変更でコードがどのような影響を受けるか表しています。
// improved code
var source = document.getElementById("location");
var target = document.getElementById("second");
var triggerValue = "loc5";
source.addEventListener("change", function () {
var selectedValue = this[this.selectedIndex].value;
if (selectedValue === triggerValue) {
target.className = "show";
} else {
target.className = "";
}
}, false);
addEventListenerラインをアクティブにすると、すべてのテストにパスします。
注：テストコードでは、異なるonchange方法をテストする場合のアプローチの変更を簡単にするために、関数をtoggleShowOnSelectedValueと命名しました。
//source.onchange = toggleShowOnSelectedValue;
source.addEventListener("change", toggleShowOnSelectedValue, false);
上のCodePenで、コメントアウト行を切り替えて、なにが起こるか試してみましょう。
クラス処理の改善
コードに関する別の問題は、classNameが以前あったものを置き換えることが原因で、2番目の<select>要素が持っていたかもしれない以前のクラスを失うことです。
// bad code
target.className = "show";
この例では、indentのクラスが表示された後もselect要素に残ってしまう問題を確認できます。
it("should retain any existing class names that were on the target element", function () {
changeSelectTo(source, "loc2");
target.classList.add("indent");
expect(target.classList.contains("indent")).toBe(true);
changeSelectTo(source, "loc5");
expect(target.classList.contains("indent")).toBe(true); // fails
});
classNameは全体のクラス名を置き換えるので、ほかのクラスもすべて削除されます。
失敗したテストをCodePenで確認できます。Jasmine固有のコードは別のPenにあることに注意してください。
こうした問題を抱える代わりに、クラス名を追加および削除するclassListを使用できます。
// good code
target.classList.add("show");
// ...
target.classList.remove("show");
次に示されるように、これでテストをパスできます。
改良後のコードは次のとおりです。
// improved code
var source = document.getElementById("location");
var target = document.getElementById("second");
var triggerValue = "loc5";
source.addEventListener("change", function () {
var selectedValue = this[this.selectedIndex].value;
if (selectedValue === triggerValue) {
target.classList.add("show");
} else {
target.classList.remove("show");
}
}, false);
IE9や古いブラウザーのサポートが理由でclassList APIの使用が心配な場合は、代わりに同様の結果を得られるaddクラスやremoveClass関数が使用できます。
最後に
コードの改善は、ハードで困難な作業ではありません。
コードの汎用化の原則は、テスト駆動開発から生じる有益な副作用です。コードの汎用化テクニックはコードをより柔軟にできるので、テストするかどうかに関わらず有益です。これは、頻繁にコードを修正する作業から多くの人を解放します。
※本記事はDan Princeが査読を担当しています。最高のコンテンツに仕上げるために尽力してくれたSitePointの査読担当者のみなさんに感謝します。
（原文：JavaScript Refactoring Techniques: Specific to Generic Code）
［翻訳：柴田理恵／編集：Livit］
