あなたはとある業務用 Web アプリケーションの開発・保守を任されています。
このアプリケーションは ASP.NET を用いて作成されており、
クライアントサイドは一部 jQuery を利用してナウなヤングにバカウケの UI を実装しています。
さて、今回は
という機能を実装することになりました。
これ自体はちゃちゃっと実装し、以下のようなHTMLが生成されるようにしました:
...
<table>
<tr>
<th>A</th>
<th>B</th>
<th>変更日時</th>
<th>変更者</th>
</tr>
<tr>
<td><input type="radio" name="new_version" value="9"/></td>
<td></td>
<td>2012-06-18 18:47:07</td>
<td>FGH</td>
</tr>
<tr>
<td><input type="radio" name="new_version" value="8"/></td>
<td><input type="radio" name="old_version" value="8"/></td>
<td>2012-05-17 08:47:02</td>
<td>DEF</td>
</tr>
...
<tr>
<td></td>
<td><input type="radio" name="old_version" value="1"/></td>
<td>2012-04-16 01:44:50</td>
<td>ABC</td>
</tr>
</table>
<input type="submit" value="AとBの差分を表示する"/>
また、特定のバージョンでの変更点を簡単に確認できるよう、
「Aの列のラジオボタンを選ぶと同じ行より一つ下にあるBの列のラジオボタンを自動で選ぶ」
という補助を実装することになりました。
これは特に難しい作業ではないので、
クライアントサイドの改修にも慣れてもらおうと考え、
今までサーバーサイドしか触れていなかったメンバーに担当してもらうことにしました。
30分後 ──
$(document).ready(function () {
// seq: シーケンス番号
$.each(["new_version", "old_version"], function () {
$("input[name='" + this + "']").each(function (idx, elem) {
if (idx == 0) $(elem).attr('checked', 'checked');
$(elem).attr('seq', idx);
});
});
$("input[name='new_version']").change(function () {
var seq = parseInt($(this).attr('seq'));
$("input[name='old_version']").eq(seq).attr('checked', 'checked');
});
});
これはまたリファクタリングのしがいのあるコードです。
ちょっと本腰を入れて直すことにしましょう。
まず「対応するラジオボタン」を選ぶための処理が臭います。
上の行のものから順に番号を振っているのですが、
この番号を紐付けるために $.attr
を使っています。$.attr
は対象の要素の属性の値を参照/設定するためのものであって、
設定するならば属性の名前や値は HTML 的に妥当なものにすべきです。
$.data
という、任意の要素に任意のデータを紐付けるための API が用意されているので、
ここではそれを使うべきです。また、 $.data
を使えば parseInt
等をする必要もなくなります。
$(document).ready(function () {
// seq: シーケンス番号
$.each(["new_version", "old_version"], function () {
$("input[name='" + this + "']").each(function (idx, elem) {
if (idx == 0) $(elem).attr('checked', 'checked');
$(elem).data('seq', idx);
});
});
$("input[name='new_version']").change(function () {
$("input[name='old_version']").eq($(this).data('seq')).attr('checked', 'checked');
});
});
次に $.each
でのループ処理ですが、
という2つの処理が混ざっています。
これは jQuery がどうこう言う以前の問題です。
混乱を避けるために各処理を分割しておきます。
$(document).ready(function () {
var $new_versions = $("input[name='new_version']");
var $old_versions = $("input[name='old_version']");
$new_versions.eq(0).add($old_versions.eq(0)).attr('checked', 'checked');
// seq: シーケンス番号
$.each(["new_version", "old_version"], function () {
$("input[name='" + this + "']").each(function (idx, elem) {
$(elem).data('seq', idx);
});
});
$("input[name='new_version']").change(function () {
$("input[name='old_version']").eq($(this).data('seq')).attr('checked', 'checked');
});
});
さて、「対応するラジオボタン」を得るための処理なのですが、どうにもまどろっこしい感じが拭えません。$.data
で任意のデータを紐付けられるのですから、
わざわざ添字を使わなくとも、
直接「対応するラジオボタン」を紐付ければ良いのです。
そうすれば「seq: シーケンス番号」という謎のコメントを書くこともなくなります。
$(document).ready(function () {
var $new_versions = $("input[name='new_version']");
var $old_versions = $("input[name='old_version']");
$new_versions.eq(0).add($old_versions.eq(0)).attr('checked', 'checked');
$new_versions.each(function (i0) {
$(this).data('pair', $old_versions.eq(i0));
});
$("input[name='new_version']").change(function () {
$(this).data('pair').attr('checked', 'checked');
});
});
今回の場合は(2)から(3)になる過程でごっそり削除していまいましたが、
$("input[name='" + this + "']")
.each(function (idx, elem) {
...
のように動的にセレクターを作成しているのも良くありません。
この書き方では this
の値に妙な文字が混ざっていると破綻します。
例えば this の値が "'foo'"
の場合、上記のコードは
$("input[name='" + this + "']")
.each(function (idx, elem) {
...
と書いたものと同じになります。$.filter
で絞り込めばこのような事態には遭遇しません:
var targetName = ...;
$("input")
.filter(function () {return $(this).attr("name") == targetName;})
.each(function (idx, elem) {
...
今回の場合は $.data
を知っているかどうかで読み易いコードを書けるかどうかが変わってきます。
不慣れなライブラリーでも API リファレンスには一通り目を通しましょう。