by ryou

jquery.tile.jsを読んで書きなおしてみた

年末年始休みに、何か建設的な事をやろうと仕事で使っているプラグインを読むことにしました。

今回読んだのは、仕事で頻繁に使っていて、尚且つサクッと読めるくらい超短いソースということでjquery.tile.jsを選択。

元ソース

/**
 * Flatten height same as the highest element for each row.
 *
 * Copyright (c) 2011 Hayato Takenaka
 * Dual licensed under the MIT and GPL licenses:
 * http://www.opensource.org/licenses/mit-license.php
 * http://www.gnu.org/licenses/gpl.html
 * @author: Hayato Takenaka (https://github.com/urin/jquery.tile.js)
 * @version: 1.1.1
 **/
(function($) {
  $.fn.tile = function(columns) {
    var tiles, $tile, max, c, h, remove, s = document.body.style, a = ["height"],
      last = this.length - 1;
    if(!columns) columns = this.length;
    remove = s.removeProperty ? s.removeProperty : s.removeAttribute;
    return this.each(function() {
      remove.apply(this.style, a);
    }).each(function(i) {
      c = i % columns;
      if(c == 0) tiles = [];
      $tile = tiles[c] = $(this);
      h = ($tile.css("box-sizing") == "border-box") ? $tile.outerHeight() : $tile.innerHeight();
      if(c == 0 || h > max) max = h;
      if(i == last || c == columns - 1) {
        $.each(tiles, function() { this.css("height", max); });
      }
    });
  };
})(jQuery);

問題点

書き方がC言語風

変数名の付け方とか、変数の宣言を頭でしているとか、書き方がC言語風味でした。

記憶が曖昧ですが、C言語は関数の途中で変数宣言が確か出来なかったはずでしたかね?C言語ではしょうがないのですが、そのような制限がない言語だと使う場所で変数の宣言をしたほうがわかりやすいのでそのようにしたほうが良さ気ですね。

参照:ローカル変数をメソッドの冒頭でまとめて宣言する慣習 | mxProject

jQueryにある機能を再度作ってしまっている

16行目の

remove = s.removeProperty ? s.removeProperty : s.removeAttribute;

は、ブラウザ間の差異に対応するための処理と思うのですが、tile.jsはjQueryを前提としており、jQueryにこの問題を解決するためのメソッドが用意されているので、そちらを使用するべきかなと思います。

他にも、22行目でtiles配列にオブジェクトを追加し、26行目でtiles配列の要素に対してjQueryメソッドを適用するようなネイティブ配列を使用した処理を挟んでいますが、これはjQueryのコレクション機能を利用して22行目を

$tiles = $tiles.add($(this));
$tile  = $(this);

のようにすれば26行目は

$tiles.css('height', max);

で済むはずです。

書き直した

部分部分を書きなおしてみる程度の考えだったのですが、いざ書きなおしてみるとほぼまるごと書き換わっちゃいました…

自分がtile.jsをリファクタリングするとしたら以下の様な感じです。

(function($) {
  $.fn.tile = function(columns) {

    /***********************************************************************
    初期化ここから
    **********************************************************************/
    var _self = this;

    if(!columns) columns = this.length;
    /***********************************************************************
    初期化ここまで
    **********************************************************************/


    /***********************************************************************
    関数宣言ここから
    **********************************************************************/
    // $targetsのheightを、一番高いものに合わせて揃える
    function adjustHeight($targets) {
      var maxH = 0; //$targetsの中で一番高い高さ

      // maxHの算出
      $targets.each(function(){
        var $target = $(this);

        var targetH = 0;
        if ($target.css('box-sizing') === 'border-box') {
          targetH = $target.outerHeight();
        } else {
          targetH = $target.innerHeight();
        }

        if (maxH < targetH) maxH = targetH;
      });

      $targets.css('height', maxH);
    }

    /*
    targetsArrayをsliceNum毎に分割した配列を返却
    例えば、
    sliceNum: 3
    targetsArray: [1, 2, 3, 4, 5, 6, 7]
    の場合
    [ [1, 2, 3], [4, 5, 6], [7] ]
    を返却する
    */
    function getSlicedArray(sliceNum, targets) {
      var outArray = [];

      while(true) {
        var res = targets.splice(0, sliceNum);
        if (res.length === 0) break;
        outArray.push(res);
      }

      return outArray;
    }

    // getSlicedArrayの中身がjQueryオブジェクト版
    function getSlicedJQArray(sliceNum, $targets) {
      var targetsArray = $.makeArray($targets);
      var outArray = getSlicedArray(sliceNum, targetsArray);

      outArray.map(function(){
        return $(this);
      });

      return outArray;
    }
    /***********************************************************************
    関数宣言ここまで
    **********************************************************************/


    /***********************************************************************
    実処理ここから
    **********************************************************************/
    _self.each(function() {
      $(this).removeAttr('height');
    });

    var tmp = getSlicedJQArray(columns, $(_self));
    $.each(tmp, function(){
      adjustHeight($(this));
    });
    /***********************************************************************
    実処理ここまで
    **********************************************************************/

    return _self;
  };
})(jQuery);

元々のソースと比べて、行数はかなり多くなってしまってはいますが、読みやすくはなってるんじゃないかな?とは思っています。

以上

今回書き直したものの、仕事では書き直したものは使うつもりはありません。バグを産んでしまっている可能性も高いですしね。