Swapping dynamically populated images on hover/touch

Question

I'm using jQuery to swap images on hover. There's a main image, and a gallery of thumbs. When the thumb is hovered over/touched, the larger version of it populates the main image, and the smaller version of the original populates the thumb. This happens via swapping src and data- attributes.

This all works, but being relatively new to JS/jQuery, the code is probably a little clunky and could use a keener eye. I don't fully grasp callbacks (among other JS things) yet, so I imagine there'll be some streamlining/chuckling with that.

function large_to_thumb(imageHovered, mainLarge, mainThumb){
    $(imageHovered).attr('src', mainThumb).data('large', mainLarge);
}

function thumb_to_large(image, mainLarge, mainThumb){
    var mainImage = $('.main-image');
    // 1. put gallery image data-large into .main-large src
    mainImage.attr('src', $(image).data('large'));

    // 2. put gallery image src into .main-large data-gallery-image
    mainImage.data('gallery-image', $(image).attr('src'));
    large_to_thumb(image, mainLarge, mainThumb);
}


$('.product-gallery img').on('mouseenter touch', function(){
    var mainLarge = $('.product-image .main-image').attr('src');
    var mainThumb = $('.product-image .main-image').data('gallery-image');
    thumb_to_large(this, mainLarge, mainThumb);
});

li {
  display:inline;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="product-image">
    <img class="main-image" src="http://placehold.it/300x300" data-gallery-image="http://placehold.it/150x150">
    <div class="product-gallery">
      <ul>
        <li>
          <img src="http://placehold.it/149x149" data-large="http://placehold.it/299x299">
        </li>
        <li>
          <img src="http://placehold.it/148x148" data-large="http://placehold.it/298x298">
        </li>
      </ul>
    </div>
  </div>

Answer 1

Some good suggestions already from @konijn, so I won't rehash any of the content from that answer.

I would consider writing this a a jQuery plugin. This will allow you to properly namespace your code and allow you to define your own variables for re-use within this scope (i.e. to prevent need to re-query DOM with every event action. This will also make your solution re-usable across various websites and DOM configurations.

That might look like

(function( $ ) {

    $.fn.galleryThumbSwapper = function(options) {

        // Extend our default options with those provided.
        // Note that the first argument to extend is an empty
        // object – this is to keep from overriding our "defaults" object.
        var opts = $extend({},$.fn.galleryThumbSwapper.defaults, options);

        // do our DOM selections once
        // define wrapper based on element this function was called on
        var $wrapper = $(this);

        // define main image and gallery image items
        // here we only find element within the wrapper such that
        // you could use multiple galleries on a page without
        // worry about interaction problems between galleries so
        // long as each gallery is contained within the wrapper
        var $mainImage = $wrapper.find(opts.mainImageSelector);
        var $galleryImages = $wrapper.find(opts.galleryImageSelector);

        // set click handler
        $galleryImages.on('click', swapImages);

        // define callback for click handler
        function swapImages() {
            // wrapper for triggered element
            $triggerElement = $(this);

            // get current main image info
            var currentMainSrc = $mainImage.attr('src');
            var currentMainData = $mainImage.data('gallery-image');

            // get triggered thumbnail info
            var newMainSrc = $triggerElement.data('large');
            var newMainData = $triggerElement.attr('src');

            // perform the swap
            $mainImage.attr('src', newMainSrc);
            $mainImage.data('gallery-image', newMainData);

            $triggerElement.attr('src', currentMainSrc);
            $triggerElement.data('large', currentMainData);
        }

        // return this for use in chaining
        return this;
    }

    // define default values for plug-in
    // you could also possibly add default values for the data fields here
    // so as to make these configurable
    $.fn.galleryThumbSwapper.defaults = {
        mainImageSelector = '.main-class',        
        galleryImageSelector = '.product-gallery img'
    }; 
}( jQuery ));


// usage
$(document).ready(
    $('.product-image').galleryThumbSwapper();
);

// or, with overriding default options
$(document).ready(
    $('.product-image').galleryThumbSwapper({
        mainImageSelector = '#main_img',        
        galleryImageSelector = '.thumb_img'            
    });
);

A few other thoughts:

• You might consider using id instead of or in addition to class for the main wrapper DOM element. i.e. product-image. This would guarantee uniqueness, particularly if you used the jQuery plug-in approach I am suggesting.
• You might consider your data attribute naming of 'large' which doesn't seem to make much sense as what you are storing in there is a URL. I actually don't see why you can't use the same data-attribute name on the main image and the gallery images.
• Seems like a bit of an odd user experience to have thumbnails potentially be changed in order based on how the user triggers these image swaps. To me as a user, I would expect to be able to hover across the thumbs, without it impacting the order of the thumbnails.
• Consider having images have their own class as well. This would address current inconsistency in having some elements be included in logic based on class while others become part of the logic based on element type.

Answer 2

This looks actually pretty good.

• Try to stick to lowerCamelCase for JavaScript, so largeToThumb instead of large_to_thumb.
• One liner functions like large_to_thumb who are only called once, should probably get inlined
• I like the variable name imageHovered, you should name the first parameter of thumb_to_large that way
• In one place you use .product-image, in the other places (including HTML) you don't, that's a bit confusing
• The more I think about it, the more you probably wanted to just pass $('.product-image .main-image') to thumb_to_large, because now you are re-getting that main image in the called function..
• Avoid getting the same selection multiple times, query selectors or some of the slowest operators.

I tried to rewrite your code, and I am missing something, but you should get the gist. Feel free to point out where I went wrong.

function large_to_thumb(imageHovered, mainLarge, mainThumb){
    $(imageHovered).attr('src', mainThumb).data('large', mainLarge);
}

function thumb_to_large( hoveredImage ){
    var $mainImage = $('.product-image .main-image'),
        $hoveredImage = $(hoveredImage),
        mainLarge = $mainImage.attr('src'),
        mainThumb = $mainImage.data('gallery-image'),
        newMainLarge  = $hoveredImage.data('large'),
        newMainGalleryImage = $hoveredImage.data('src');

    // 1. put gallery image data-large into .main-large src
    $mainImage.attr('src', newMainLarge );
    // 2. put gallery image src into .main-large data-gallery-image
    $mainImage.data('gallery-image', newMainGalleryImage);
    // 3. update the hovered image to show what was in the main
    $hoveredImage.attr('src', mainThumb)
                 .data('large', mainLarge);
}


$('.product-gallery img').on('mouseenter touch', function(){
    thumb_to_large(this);
});

li {
  display:inline;
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div class="product-image">
    <img class="main-image" src="http://placehold.it/300x300" data-gallery-image="http://placehold.it/150x150">
    <div class="product-gallery">
      <ul>
        <li>
          <img src="http://placehold.it/149x149" data-large="http://placehold.it/299x299">
        </li>
        <li>
          <img src="http://placehold.it/148x148" data-large="http://placehold.it/298x298">
        </li>
      </ul>
    </div>
  </div>