質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう！

質問する

ただいまの
回答率

88.33%

いったい何が変わったのか

解決済

回答 1

投稿

  • 評価
  • クリップ 0
  • VIEW 1,274
aaaaaaaa

aaaaaaaa

score 481

以下のjavascriptは、ブレイクスルーjavascript34頁から49頁に記載されている頁上に存在する画像をモーダルウィンドウで拡大表示するソースです。
showメソッドとModalのプロトタイプオブジェクトである、createCounter()、next()、prev()に対して変更を加えております。
show()の変数をプロパティからローカル変数に変更したことで、show以外から接続できなくなり、書き換えられる心配がないことは、書籍に書いており、なるほどと思いましたが、
それ以外に、ソースを変更したことでどのような利点や変更した理由があるのでしょうか。 

function Modal(el) {
    this.initialize(el);
}

Modal.prototype.initialize = function(el) {
    this.$el = el;
    this.$container = $("#modal");
    this.$contents = $("#modal-contents");
    this.$close = $("#modal-close");
    this.$next = $("#modal-next");
    this.$prev = $("#modal-prev");
    this.$overlay = $("#modal-overlay");
    this.$window = $(window);
    this.handleEvents();
};

Modal.prototype.handleEvents = function() {
    var self = this;

    this.$el.on("click", function(e) {
        self.show(e);
        return false;
    });    

    this.$close.on("click",function(e) {
        self.hide(e);
        return false;    
    });

    this.$overlay.on("click",function(e) {
        self.hide(e);
        return false;
    });

    this.$next.on("click", function(e) {
        self.next(e);
        return false;
    });

    this.$prev.on("click", function(e) {
        self.prev(e);
        return false;
    });
};

Modal.prototype.show = function(e) {

    var $target = $(e.currentTarget),
        src=$target.attr("href");

    this.$contents.html("<img src=\"" + src +"\"/>");
    this.$container.fadeIn();
    this.$overlay.fadeIn();
    //変更後
    var index = $target.data("index");
    this.countChange = this.createCounter(index,this.$el.length);
    //変更後ここまで
    return false;

};

Modal.prototype.hide = function(e) {
    this.$container.fadeOut();
    this.$overlay.fadeOut();
};

Modal.prototype.slide = function(index) {
    this.$contents.find("img").fadeOut({
        complete: function() {
            var src = $("[data-index=\"" + index + "\"]").find("img").attr("src");
            $(this).attr("src",src).fadeIn();
        }
    });
};
//変更後
Modal.prototype.createCounter = function(index, len) {
    return function(num) {
        return index = (index + num + len) % len;
    };
};

Modal.prototype.next = function() {
    this.slide(this.countChange( 1 ));
};

Modal.prototype.prev = function() {
    this.slide(this.countChange( -1 ));
};
//変更後ここまで            
var modal = new Modal($("#modal-thumb a"));
//ブレイクスルーjavascript chapter2


変更前のソース 

//<中略>
Modal.prototype.show = function(e) {

    var $target = $(e.currentTarget),
        src=$target.attr("href");

    this.$contents.html("<img src=\"" + src +"\"/>");
    this.$container.fadeIn();
    this.$overlay.fadeIn();
    //変更前
    this.index = $target.data("index");
    //変更前ここまで
    return false;

};
//<中略>
//変更前
Modal.prototype.countChange = function(num,index,len) {
    return (index+num+len) % len;
};
Modal.prototype.next = function() {
    this.index = this.countChange(1,this.index,this.$el.length);
    this.slide(this.index);
};
Modal.prototype.prev = function() {
    this.index = this.countChange(-1,this.index,this.$el.length);
    this.slide(this.index);
};
//変更前ここまで
<!DOCTYPE html>
<html lang="ja">
<head>
<meta charset="utf-8">
<title>chapter2</title>
<link rel="stylesheet" href="./index.css" />
</head>
<body>
<div id="container">
  <ul id="modal-thumb">
    <li><a href="photo-01.jpg" data-index="0">
    <img alt="" src="photo-01.jpg"></a></li>
    <li><a href="photo-02.png" data-index="1">
    <img alt="" src="photo-02.png"></a></li>
    <li><a href="photo-03.png" data-index="2">
    <img alt="" src="photo-03.png"></a></li>
  </ul>
</div>

<div id="modal">
  <div id="modal-container">
   <!-- モーダルで表示する拡大画像を入れるための要素 -->
    <div id="modal-contents"></div>
    <!-- モーダルウィンドウの右上に表示するボタン -->
    <div id="modal-close"><span class="glyphicon glyphicon-remove" aria-hidden="true"></span></div>
    <div id="modal-prev"><span class="glyphicon glyphicon-chevron-left" aria-hidden="true"></span></div>
    <div id="modal-next"><span class="glyphicon glyphicon-chevron-right" aria-hidden="true"></span></div>
  </div>
</div>

<div id="modal-overlay"></div>
<script src="http://code.jquery.com/jquery-1.11.1.min.js"></script>
<script src="./index.js"></script>
<script>
</script>
</body>
</html>
@charset "utf-8";

ul, li {
    padding: 0;
    list-style: none;
}
#container {
    margin-top: 20px;
}
#modal {
    /* #modalと#modal-overlyは、サムネイルをクリックした時に表示されるものなので非表示にしている */
    display: none;
    position: absolute;
    top: 0;
    left: 0;
    right: 0;
    bottom: 0;
    margin: auto;/*autoにすると自動的に適切な余白が設定される。widthを指定したうえで左右の余白をautoにすると、ボックスは水平方向の中央に揃う。*/
    width: 750px;
    height: 500px;/*500px*/
    box-shadow: 0 4px 12px rgba(0,0,0,0.5);
    border-radius: 6px;/*6px*/
    overflow: hidden;
    /* #modalは#modal-overlyよりも前面に表示させたいので、z-indexプロパティで重ね順を調整する。 */
    z-index: 10;
}
#modal-container {
    position: relative;/*要素が通常配置される位置を基準にして要素を配置する際に指定*/
    background: #FFF;/*#FFF*/
    width: 100%;
    height: 100%;
    overflow: hidden;
}
#modal-contents {
    width: 100%;
    height: 100%;
}
#modal-close, #modal-prev, #modal-next {
    position: absolute;
    top: 10px;
    padding: 8px;
    font-size: 8px;
    line-height: 1;/*行の高さを指定する。単位を指定していないときは、フォントの大きさに指定した数値を乗算した数値が行の高さとなる。つまり8px*2だ。*/
    border-radius: 50%;
    background: #fff;
    box-shadow: 0 0 1px rgba(0,0,0,0.4);/*長さ(正数だとボックス右側面と右下、負数だと左側面と左上))、ぼかし半径、広がり、*/
    cursor: pointer;
}
#modal-close {
    background: #ff6353;
    right: 10px;
}
#modal-prev {
    right: 80px;
}
#modal-next {
    right: 50px;
}

#modal-overlay {
    /* #modalと#modal-overlyは、サムネイルをクリックした時に表示されるものなので非表示にしている */
    display: none;
    position: absolute;
    top: 0;
    left: 0;
    width: 100%;
    height: 100%;
    background: rgba(0,0,0,0.4);
    cursor: pointer;
}
#modal-contents img {
    max-width: 100%;
}
#modal-thumb {
    text-align: center;
}
#modal-thumb li {
    display: inline-block;
    width: 160px;
    margin-left: 5px;
    margin-bottom: 5px;
}
#modal-thumb li img {
    width: 100%;
}

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 1 件

link

checkベストアンサー

+1

コードのメンテナンス性が上がっていると思います。
たとえば、this.$el.lengthを別の値に変更したくなった時に、
変更前コードは2か所変更する必要があります。 

Modal.prototype.next = function() {
    this.index = this.countChange(1,this.index,this.$el.length);  // ここ
    this.slide(this.index);
};
Modal.prototype.prev = function() {
    this.index = this.countChange(-1,this.index,this.$el.length);  // ここ
    this.slide(this.index);
};


変更後コードは1か所の変更で済みます。 

this.countChange = this.createCounter(index,this.$el.length);  // ここ

可読性とのトレードオフになることも多いのですが、同じコードを繰り返すことは避けた方がメンテナンス性が上がりますし、バグも減るかと思います。

蛇足ですが。
ここは私ならこう書きます。 

    var index = $target.data("index");
    this.countChange = this.createCounter(index,this.$el.length);


↓ 

    this.countChange = this.createCounter($target.data("index"),this.$el.length);


ローカル変数に持たせていますが特に他から参照をしているわけでもなさそうですし、そもそもクロージャにしているindexと参照も異なってしまっています。バグの原因になりそうな？

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/07/18 19:45

    ご回答ありがとうございました。

    >>ローカル変数に持たせていますが特に他から参照をしているわけでもなさそうですし、そもそもクロージャにしているindexと参照も異なってしまっています。バグの原因になりそうな？
    重ね重ね申し訳ありませんが、読点より前は、つまり関数内関数からローカル変数indexを参照していない、ということでしょうか。間違っていたら申し訳ありません。
    読点よりあとの「そもそもクロージャにしているindexと参照も異なってしまっています。」というのは、$targetに設定された値とプロパティをdataメソッドで取得すると、何らかの理由で参照が異なってしまい、不具合の原因になるということでしょうか。

    キャンセル

  • 2017/07/18 19:55

    前者についてはおっしゃる通りです。このコードの中でこのindex変数をどこでも参照していないので、保持する必要はないだろう、ということです。

    後者については、なんといいますか、そのindex変数はいかにもスライダーの位置を保持していそうだけど、実はモーダルを開いた時のインデックス値を保持しているので、なんか間違えそうだなあ、ということです。変数名を変えるのもいいかもしれませんね。

    キャンセル

15分調べてもわからないことは、teratailで質問しよう！

  • ただいまの回答率 88.33%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる
質問する

関連した質問

同じタグがついた質問を見る

SNSアカウントでログイン

teratailアカウントでログイン

思考するエンジニアのためのQ&Aサイト「teratail」について詳しく知る