JSのコードを短く、簡潔にしたい

Question

サムネイルをクリックすると指定の場所に大きく画像が表示されるものです。
これだけでも実際に動きますが、もっとJSのコードを短く簡単に書く方法はないでしょうか？
よろしくお願いいたします。

$(function() {
    var i = 1;

    $("div.hogecol").each(function(){
        $(this).addClass('hoge_' + (i++));

        $('.hoge_1').css('cursor','pointer').find('img').click(function(){
          var $thisImg = $(this).attr('src');
          $('.hogeImg1').attr({src:$thisImg});
         });
        $('.hoge_2').css('cursor','pointer').find('img').click(function(){
          var $thisImg = $(this).attr('src');

          $('.hogeImg2').attr({src:$thisImg});
         });
        $('.hoge_3').css('cursor','pointer').find('img').click(function(){
          var $thisImg = $(this).attr('src');

          $('.hogeImg3').attr({src:$thisImg});
         });
     });
 });

<img src="test1.jpg" alt="" class="hogeImg1">
<div class="hogecol">
 <img src="sample1.jpg" alt="">
 <img src="sample2.jpg" alt="">
</div>

<img src="test2.jpg" alt="" class="hogeImg2">
<div class="hogecol">
 <img src="sample3.jpg" alt="">
 <img src="sample4.jpg" alt="">
</div>

<img src="test3.jpg" alt="" class="hogeImg3">
<div class="hogecol">
 <img src="sample5.jpg" alt="">
 <img src="sample6.jpg" alt="">
</div>

Comments

[kszk311]

記載のソースの中に「.hoge_1」「. hoge_2」「. hoge_3」の要素がありませんが…。

[kszk311]

失礼、「$(this).addClass('hoge_' + (i++));」で作成してたのですね。

Answer 1

なるほど、今回の課題は.hogeImg1ですね。
この1を上手くループに出来ないから毎回書く必要があったのですね。

$("div.hogecol").each(function(){

なんでここでeach使えてるのに、他で使いまわす案が出てこないんですか！

外でvar iと宣言した後にひたすらインクリメントで使いまわしていますが、
each時の第一引数に0から順番の添字が入ってくるのでそちらを使った方が100万倍かっこいいです。
しかも、eachの度にhoge_1〜hoge_3全てに同じ事を毎回やってるじゃないですか！

クリックの中の$thisImgもダメです。
これはsrcの中身のStringでしょ？だから変数名はsrcのようなものであるべきです。

というわけで、上記の気になった箇所もまとめてリファクタリングします。

$(function() {
  $("div.hogecol").each(function(i){
    var index = i + 1;
    var $col = $(this);
    $col.addClass('hoge_' + index); // これまだいる？
    $col.css('cursor', 'pointer'); // これはCSSで書いた方が良いと思う
    $col.find('img').click(function(){
      var src = $(this).attr('src');
      $('.hogeImg' + index).attr({src: src});
    });
  });
});

ざっとこんな感じですかね。
これで意図踊りの動作になるか確認をお願いします。

Comments

[southern_flavor]

ご回答ありがとうございます。
eachの正しい使い方、変数名のつけ方、たいへん勉強になりました。

Answer 2

位置関係で判断してよいのであればこう

$(function() {
  $('.hogecol img').css('cursor','pointer').on('click',function(){
    $(this).closest('.hogecol').prev('img').attr('src',$(this).attr('src'));
  });
});

Comments

[southern_flavor]

ご回答ありがごうございます。
シンプルで、参考にさせて頂きたいと思います。

Answer 3

ほとんどyambejpさんと同じになってしまうのですが、
JSの短さにこだわるなら、CSSで書ける部分はCSS側に書いてしまうのがいいかと

	.hogecol img {
		cursor: pointer:
	}

$(function() {
 	$('.hogecol img').click(function(){
		$(this).closest('.hogecol').prev('img').attr('src',$(this).attr('src'));
	});
});

Comments

[southern_flavor]

ご回答ありがとうございます。
確かにCSSの指定は別途、cssで対応したほうがよさそうですね。
こちらも参考にさせていただきます。