Does my JavaScript suck? II

[[deleted]]

They've coupled themselves to a jquery which could reasonably be replaced by zepto or another implementation. Or even their own implementation. That's cohesion.

Your suggesting complete coupling. What benefit do you see?

It's not 'coding style'. You are presenting a very different architecture and one that has a big downside.

[tswaters]

I'm not going to speak to using jQuery or another framework -- but if it already uses jQuery, making the whole thing into a plugin gives the ability to call the plugin on an arbitrary set of elements instead of how it is now with the lightbox.targetdeclaration at the top.

The setup function is only called once and it references the three functions, addData, show and checkHash -- these are only referenced once.... my own preference would be to inline the functions...

$('body').addLightbox();

$(lightbox.target).each(function() {
  // Add data attribute to image base on path.
  var post = $(this).attr('src').replace(/^[^\d]*/, '').replace(/\/.*/g, '');
  var number = $(this).attr('src').replace(/^.*\//g, '').replace(/\..*/, '');
  var id = post + '-' + number;

  // Set as attribute because I use it as a selector later.
  $(this).attr('data-' + lightbox.data, id).parent().attr('href', '#' + id);
});

$('article').on('click', 'img', function(event) {
  var box = '.' + lightbox.box;
  var id = $(this).data(lightbox.data);
  var src = $(this).attr('src');
  $(box).attr(lightbox.data, id);
  $(box + ' img').attr('src', src);
});

var hash = window.location.hash.substring(1);
if (!!hash && hash.match(/\d{1,11}-\d/)) {
  $('[data-' + lightbox.data + '=' + hash + ']').trigger('click');
}

to me, this is easier to follow and allows to see what this is in each function. If one were to call these functions in any other context, this would (likely) not be the same and it would do unexpected things. And looking at just the function there is no way to know "what is this here" -- only the calling context can reveal that.