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 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.