patternjavascriptMinor
Underscore.js and jQuery templating
Viewed 0 times
underscoretemplatingandjquery
Problem
I want to improve my code or structure. How does it look? Any comments or suggestions?
$(function() {
function searchGit() {
//grab value of search field
var search = $('#search').val();
// Validates entered value
if (search.length > 0) {
//pull json data from url
$.ajax({
url: 'https://api.github.com/legacy/repos/search/' + search,
dataType: 'json',
success: function(data) {
// use the results to template the html using _
var results = data.repositories,
resultsTemplate = _.template($("#results-template").html()),
resultingHtml = resultsTemplate({results : results});
// place the generated data into the html
$("#results-container").html(resultingHtml);
}
});
} else {
alert("Please enter repo name");
}
return false;
}
$('#submit').on('click', searchGit);
});Solution
I would only change a few things around. First, this is all one big monolithic function. Break it apart into smaller individual functions and try to have each function do one job only.
Second, I would convert from using the
Next, I would move all of the individual functions out of the
Eventually, I would recommend replacing all of the DOM selectors with variables. That way it would be less coupled together. For example, in the process data function:
Hope that helps.
Second, I would convert from using the
success callbacks to using Promises. That might be a matter of taste; Feel free to ignore. Third, I would encapsulate your code inside an IIFE to create a private scope. It also insures that $ is going to be jQuery. Next, I would move all of the individual functions out of the
document.ready event. There is no reason to wait until then to load the functions. Lastly, I would change to using the submit event instead of the click event. I would guess that is really when you need to do your call. (function( $ ) {
// IIFE for a private scope
function getData( search ) {
return $.ajax({
url: 'https://api.github.com/legacy/repos/search/' + search,
dataType: 'json'
});
}
function searchGit( event ) {
//grab value of search field
event.preventDefault();
var search = $('#search').val();
// Validates entered value
if ( search.length ) {
$.when( getData( search ) ).done( function( data ) {
processData( data );
});
/* could also be written like this:
var call = getData( search );
call.done( function( data ) { ... });
OR
getData( search ).done( function( data ) { ...});
*/
} else {
alert( "Please enter repo name" );
}
return false;
}
function processData( data ) {
// use the results to template the html using _
var results = data.repositories,
resultsTemplate = _.template($("#results-template").html()),
resultingHtml = resultsTemplate({results : results});
// place the generated data into the html
$("#results-container").html( resultingHtml );
}
$(function(){
$('#submit').on( 'submit', searchGit );
});
})( jQuery );Eventually, I would recommend replacing all of the DOM selectors with variables. That way it would be less coupled together. For example, in the process data function:
function processData( data , templateName, resultsElement ) {
// use the results to template the html using _
var results = data.repositories,
resultsTemplate = _.template( $( templateName ).html() ),
resultingHtml = resultsTemplate( {results : results} );
// place the generated data into the html
$( resultsElement ).html( resultingHtml );
}Hope that helps.
Code Snippets
(function( $ ) {
// IIFE for a private scope
function getData( search ) {
return $.ajax({
url: 'https://api.github.com/legacy/repos/search/' + search,
dataType: 'json'
});
}
function searchGit( event ) {
//grab value of search field
event.preventDefault();
var search = $('#search').val();
// Validates entered value
if ( search.length ) {
$.when( getData( search ) ).done( function( data ) {
processData( data );
});
/* could also be written like this:
var call = getData( search );
call.done( function( data ) { ... });
OR
getData( search ).done( function( data ) { ...});
*/
} else {
alert( "Please enter repo name" );
}
return false;
}
function processData( data ) {
// use the results to template the html using _
var results = data.repositories,
resultsTemplate = _.template($("#results-template").html()),
resultingHtml = resultsTemplate({results : results});
// place the generated data into the html
$("#results-container").html( resultingHtml );
}
$(function(){
$('#submit').on( 'submit', searchGit );
});
})( jQuery );function processData( data , templateName, resultsElement ) {
// use the results to template the html using _
var results = data.repositories,
resultsTemplate = _.template( $( templateName ).html() ),
resultingHtml = resultsTemplate( {results : results} );
// place the generated data into the html
$( resultsElement ).html( resultingHtml );
}Context
StackExchange Code Review Q#88526, answer score: 3
Revisions (0)
No revisions yet.