patternjavascriptMinor
Improving logic for pulling and passing data();
Viewed 0 times
passinglogicforandpullingdataimproving
Problem
I'm pulling data from a list of images and passing it into an information container after click. After looking at my logic I assume there is a better way to do this. Any ideas would be helpful.
var getData = function() {
$('.guest-image').html('');
if ($($this).data("known1") != undefined) {
$('.guest-data .known-for li').eq(0).html('');
}
if ($($this).data("known2") != undefined) {
$('.guest-data .known-for li').eq(1).html('');
}
if ($($this).data("known3") != undefined) {
$('.guest-data .known-for li').eq(2).html('');
}
$('.guest-data .title').text($($this).data("name"));
$('.guest-data .desc').text($($this).data("desc"));
$('.guest-data .link').text($($this).data("link"));
$('.guest-data .link').attr('href', $($this).data("link"));
}Solution
squished and optimized, the code looks like this:
and here's the explanation
Here's a JS perf performance test. Compared to your code, the builder method is more readable, verbose but 23% slower. The string assembly method is 7% faster than your code.
Seems like it was faster only on my Firefox 18 and Firefox 21 Android (Nightly) and was too eager to conclude.
function getData() {
var guestData = $('.guest-data'),
knownFors = $('.known-for li', guestData),
data = $($this).data();
$('.guest-image').html('');
$.each(data.knowns, function (i, entry) {
knownFors.eq(i).html('')
});
$('.title', guestData).text(data.name);
$('.desc', guestData).text(data.desc);
$('.link', guestData).text(data.link).attr('href', data.link)
}and here's the explanation
//named functions are better in debugging
function getData() {
//cache frequently used elements
var guestData = $('.guest-data'),
//providing a query context limits jQuery to locate a selector only under the context
//so in this code, we find .known-for li only under guestData which we already cached
//instead of traversing the DOM again for each find
knownFors = $('.known-for li', guestData),
//jQuery data can be fetched as a JS object by a parameter-less .data()
data = $($this).data();
//by informal convention, variables prefixed by $ usually means it
//references a jQuery object. You may not need wrap it again with
//a $() call if you are sure it's already a jQuery object
//there are two ways to build elements in jQuery
//for performance, string assembly would be faster
$('.guest-image').html('');
//for readability, but slow, the jQuery element builder method
$('.guest-image').html($('', {
'src': data.image
}));
//I also have to note as to why .guest-image was not cached
//it's simply because it is only used once in the function
//for your "knowns", I suggest placing them in an array and in order
//so that you can easily iterate through and attach to your html
//example of the structure
/*
data.knowns = [
{
known : 'known here',
title : 'title here'
},{
known : 'known here',
title : 'title here'
}
];
*/
//looping through the structure
$.each(data.knowns, function (i, entry) {
//string assembly method
knownFors.eq(i).html('')
//builder method
knownFors.eq(i).html($('', {
'src': entry.known,
'data-title': entry.title
}));
});
$('.title', guestData).text(data.name);
$('.desc', guestData).text(data.desc);
//lastly, some jQuery functions return the element they operated on
//this makes chaining possible.
$('.link', guestData).text(data.link).attr('href', data.link);
}Here's a JS perf performance test. Compared to your code, the builder method is more readable, verbose but 23% slower. The string assembly method is 7% faster than your code.
Seems like it was faster only on my Firefox 18 and Firefox 21 Android (Nightly) and was too eager to conclude.
Code Snippets
function getData() {
var guestData = $('.guest-data'),
knownFors = $('.known-for li', guestData),
data = $($this).data();
$('.guest-image').html('<img src="' + data.image + '"/>');
$.each(data.knowns, function (i, entry) {
knownFors.eq(i).html('<img src="' + entry.known + '" data-title="' + entry.title + '"/>')
});
$('.title', guestData).text(data.name);
$('.desc', guestData).text(data.desc);
$('.link', guestData).text(data.link).attr('href', data.link)
}//named functions are better in debugging
function getData() {
//cache frequently used elements
var guestData = $('.guest-data'),
//providing a query context limits jQuery to locate a selector only under the context
//so in this code, we find .known-for li only under guestData which we already cached
//instead of traversing the DOM again for each find
knownFors = $('.known-for li', guestData),
//jQuery data can be fetched as a JS object by a parameter-less .data()
data = $($this).data();
//by informal convention, variables prefixed by $ usually means it
//references a jQuery object. You may not need wrap it again with
//a $() call if you are sure it's already a jQuery object
//there are two ways to build elements in jQuery
//for performance, string assembly would be faster
$('.guest-image').html('<img src="'+data.image+'"/>');
//for readability, but slow, the jQuery element builder method
$('.guest-image').html($('<img/>', {
'src': data.image
}));
//I also have to note as to why .guest-image was not cached
//it's simply because it is only used once in the function
//for your "knowns", I suggest placing them in an array and in order
//so that you can easily iterate through and attach to your html
//example of the structure
/*
data.knowns = [
{
known : 'known here',
title : 'title here'
},{
known : 'known here',
title : 'title here'
}
];
*/
//looping through the structure
$.each(data.knowns, function (i, entry) {
//string assembly method
knownFors.eq(i).html('<img src="'+entry.known+'" data-title="'+entry.title+'"/>')
//builder method
knownFors.eq(i).html($('<img/>', {
'src': entry.known,
'data-title': entry.title
}));
});
$('.title', guestData).text(data.name);
$('.desc', guestData).text(data.desc);
//lastly, some jQuery functions return the element they operated on
//this makes chaining possible.
$('.link', guestData).text(data.link).attr('href', data.link);
}Context
StackExchange Code Review Q#21006, answer score: 2
Revisions (0)
No revisions yet.