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

GitHub API based app

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
githubapibasedapp

Problem

I don't have formal training in coding, so I would like some input in my code. It is working really well, without any issues. I just wanted to see if I can somehow improve my code. I divided the major things into separate function.

Here is a working demo.

```
(function($) {
// Use _ to template the resultsTemplating details
function resultsTemplating(data, inputValue) {
var results = data,
resultsTemplate = _.template($("#results-template").html()),
resultingHtml = resultsTemplate({
results : results,
searchVal : inputValue,
amount: results.length
});

// place the generated data into the html
$("#results-container").html(resultingHtml);
}
// Use _ to template the overlay details
function overlayTemplating(data, id) {
// loop through JSON and match clicked element, then template
for(var i = 0; i < data.length; i++) {
if(data[i].created == id) {
var overlayTemplate = _.template($("#overlay-template").html()),
overlayHtml = overlayTemplate({
name : data[i].name,
username : data[i].username,
language: data[i].language,
url: data[i].url,
description: data[i].description
});

}
}
// place the generated data into the html
$("#overlay-container").html(overlayHtml);
}
// Grab Deatils of clicked node, and template it
function repoDetails(data, id) {
var container = $('#overlay-container');
container.fadeIn('fast');

overlayTemplating(data, id);

// Closes the overlay
container.find('.close').on('click', function() {
container.fadeOut('fast');
return false;
});

}
// Scroll Back to the top of the page
function ba

Solution

A small review, but your naming seems a bit repetitive.

This:

// Use _ to template the resultsTemplating details
function resultsTemplating(data, inputValue) {
    var results = data,
        resultsTemplate = _.template($("#results-template").html()),
        resultingHtml = resultsTemplate({
            results : results,
            searchVal : inputValue,
            amount: results.length
        });

    // place the generated data into the html
    $("#results-container").html(resultingHtml);
}


has results and resulting all over the place, I would go for something like this:

// Use _ to template the resultsTemplating details
function templateResults(results, searchValue) {
    var template = _.template($("#results-template").html()),
        html = resultsTemplate({
            results : results,
            searchVal : searchValue,
            amount: results.length
        });

    // place the generated data into the html
    $("#results-container").html(html);
}


I also removed the move from data to results to save a line and renamed inputValue to searchValue. I noticed that you have inputValue and searchVal, in my mind you want to be consistent and have inputValue and searchValue or inputVal and searchVal. I much prefer the fully spelled out name.

This bit is funny:

overlayHtml = overlayTemplate({
                    name : data[i].name,
                    username : data[i].username,
                    language: data[i].language,
                    url: data[i].url,
                    description: data[i].description
                });


You are creating an object with the exact same properties which already exist in data[i]. Unless you have unwanted fields in data[i] that you really dont want in your template, you can simply

overlayHtml = overlayTemplate(data[i]);


All in all, this is really great code for someone without formal training. I like the size of your functions, the level of commenting and how easy the code is to follow.

Code Snippets

// Use _ to template the resultsTemplating details
function resultsTemplating(data, inputValue) {
    var results = data,
        resultsTemplate = _.template($("#results-template").html()),
        resultingHtml = resultsTemplate({
            results : results,
            searchVal : inputValue,
            amount: results.length
        });

    // place the generated data into the html
    $("#results-container").html(resultingHtml);
}
// Use _ to template the resultsTemplating details
function templateResults(results, searchValue) {
    var template = _.template($("#results-template").html()),
        html = resultsTemplate({
            results : results,
            searchVal : searchValue,
            amount: results.length
        });

    // place the generated data into the html
    $("#results-container").html(html);
}
overlayHtml = overlayTemplate({
                    name : data[i].name,
                    username : data[i].username,
                    language: data[i].language,
                    url: data[i].url,
                    description: data[i].description
                });
overlayHtml = overlayTemplate(data[i]);

Context

StackExchange Code Review Q#88736, answer score: 3

Revisions (0)

No revisions yet.