TIM Labs

美しいプログラムを書く(業務用Webアプリケーション保守編)

| コメント(0) | トラックバック(0)

あらすじ

あなたはとある業務用 Web アプリケーションの開発・保守を任されています。 このアプリケーションは ASP.NET を用いて作成されており、 クライアントサイドは一部 jQuery を利用してナウなヤングにバカウケの UI を実装しています。

さて、今回は

  • 商品情報の変更履歴を一覧表示する。
  • 一覧から2つのバージョンを選んで差分を表示できるようにする。

という機能を実装することになりました。 これ自体はちゃちゃっと実装し、以下のような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分後 ──

(0) できあがったコード

$(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');
    });
});

これはまたリファクタリングのしがいのあるコードです。 ちょっと本腰を入れて直すことにしましょう。

(1) 独自のデータを紐付けるには $.data を使う

まず「対応するラジオボタン」を選ぶための処理が臭います。 上の行のものから順に番号を振っているのですが、 この番号を紐付けるために $.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');
    });
});

(2) 一つのループ内で複数の処理を行わない

次に $.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');
    });
});

(3) 添字ではなく要素そのものを使う

さて、「対応するラジオボタン」を得るための処理なのですが、どうにもまどろっこしい感じが拭えません。 $.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=''foo'']")
    .each(function (idx, elem) {
        ...

と書いたものと同じになります。 $.filter で絞り込めばこのような事態には遭遇しません:

var targetName = ...;
$("input")
    .filter(function () {return $(this).attr("name") == targetName;})
    .each(function (idx, elem) {
        ...

まとめ

今回の場合は $.data を知っているかどうかで読み易いコードを書けるかどうかが変わってきます。 不慣れなライブラリーでも API リファレンスには一通り目を通しましょう。

合わせて読みたい

トラックバック(0)

トラックバックURL: http://labs.timedia.co.jp/mt/mt-tb.cgi/303

コメントする

このブログ記事について

このページは、kanaが2012年7月16日 00:00に書いたブログ記事です。

ひとつ前のブログ記事は「Gitで特定のコミットを指定する108の方法」です。

次のブログ記事は「名状し難いコードを書く(コメント編)」です。

最近のコンテンツはインデックスページで見られます。過去に書かれたものはアーカイブのページで見られます。