HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Underscore.js and jQuery templating

Submitted by: @import:stackexchange-codereview··
0
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 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.