patternjavascriptMinor
GitHub API based app
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
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:
has
I also removed the move from
This bit is funny:
You are creating an object with the exact same properties which already exist in
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.
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 simplyoverlayHtml = 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.