Better way to write 'bulky' function in jQuery

Question

I have this function that works fine, but it looks really ugly and repetitive...

Basically I have a featured profile and underneath I have 4 smaller thumbnails, on click I want to populate the large profile. The code works fine it just doesn't look too good...

$('ul.teamProfiles li').click(function () { 
 var $name = $(this).find('h4.name').html();
 var $picture = $(this).find('img').data('bigimg');
 var $biography = $(this).find('.biographyParas').html();
 var $jobTitle = $(this).find('p.jobTitle').html();
 var $telephone = $(this).find('p.telephone').html();
 var $linkedin = $(this).find('a.linkedinLink').attr('href');
 var $twitter = $(this).find('a.twitterLink').attr('href');
 var $mailto = $(this).find('a.mailtoLink').attr('href');
 $('.profileDetail .largeImg').attr('src', $picture);
 $('.profileDetail h3.name').html($name);
 $('.profileDetail h4.jobtitle').html($jobTitle);
 $('.profileDetail .biographyContainer').html($biography);
 $('.profileDetail span.telephone').html($telephone);
 $('.profileDetail a.linkedin').attr('href', $linkedin);
 $('.profileDetail a.twitter').attr('href', $twitter);
 $('.profileDetail a.email').attr('href', $mailto);
});

• megawac: I don't like the a.linkedinLink etc. selectors. A rule in CSS is to use specificity so removing the target element type would be the first thing I do; i.e. .linkedin-link

Answer 1

It's difficult to really clean it up, since a lot of it is "manual" translation from one piece of markup to another. Different elements, different attributes etc.

However, there's the low-hanging fruit (and golden rule): Don't repeat yourself.

So don't call $(this) over and over. It's just inefficient. Do something like

var source = $(this),
 target = $(".profileDetail");

and then chain off of those, like source.find('h4.name')...

However, a nicer way to do this might be to use a data-source/data-target attributes on the "big" profile elements. For instance

<div class="profileDetail">
 <img class="largeImg" data-source="img, data, bigimg" data-target="src">
 <h3 class="name" data-source="h4.name"></h3>
 ...

So in each element data-source is a comma-separated list of the selector to run in the source element, the (optional, defaults to html()) method to call on the result, and the (optional) arguments. Meanwhile, data-target is can be an attribute name, but it's optional and defaults to html()

Then, in the JS, do something like this:

$('ul.teamProfiles li').click(function (event) {
 var selection = $(this),
 destination = $(".profileDetail:first");
 // find and loop through elements with a data-source attribute
 destination.find("[data-source]").each(function () {
 var target = $(this), // an element in the "big" profile container
 data = (target.data('source') || "").split(/\s*,\s*/), // parse the data-source attribute
 selector = data[0], // get the selector
 method = data[1] || 'html', // get the method (defaults to html)
 args = data.slice(2), // slice off the arguments
 attr = target.data('target'), // get the destination attribute (if any)
 source, content;
 if(!selector) return; // nothing to do
 source = selection.find(selector); // find the source element in the small profile
 content = source[method].apply(source.get(0), args); // call the method with the args
 if(attr) {
 target.attr(attr, content); // set the target's attribute, if that's what's needed
 } else {
 target.html(content); // otherwise, default to inserting the content as html
 }
 });
});

That function is pretty generic (though it won't work for more complex stuff like setting a bunch of css).

The point is, that the mapping of small profile to large profile can be declared entirely in the HTML; the JS doesn't need to change if you add/remove elements.

Haven't tested it, but it should work

• megawac: One little convention nitpick, people like to prefix $ to cached jQuery objects. E.g. var $this = $(this)
• Flambino: @megawac Sure - personally, though, I dislike it. $this isn't any more descriptive than this; I prefer to just use a more descriptive name. Whether it's a jQuery-wrapped element or not, it's just a variable. The $-prefix is a sort of Hungarian notation that I just personally don't like, unless you need to disambiguate a "raw" element and a jQuery-wrapped one. Besides, all those dollar-signs makes me think of PHP, and then I start to feel sad... Feel free to use it yourself, though :)
• megawac: Just a convention, I hated it at first. Now I typically prefix any element wrapper (e.g. a dojo element) with $ as most devs will instantly understand what the variable is
• Flambino: @megawac Oh, I get why it's a convention, and the reasons are sound enough. I just can't get myself to like it. Besides my PHP traumas, I just think that it's weird to put jQuery (or Dojo, etc.) on a pedestal where it needs its own special naming convention. Otherwise, go all-in on Hungarian notation. But then again, that doesn't make sense in a dynamically-typed language. I just rarely find it useful (or pretty) to add all those extra $; it feels more like visual noise than anything helpful. If I see one $(..) call, I can pretty much assume it's jQuery anyway.