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

JavaScript app to search GitHub using JSON API

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

Problem

I've written this simple JavaScript application for a homework assignment and I've received feedback saying it could be written better.

Can I get some feedback on how to write this differently? It works for me and returns the results I need, but I would like some more specific feedback. It was meant to be simple and not meant to require more than 2 hours on it.

The requirements were...



  • You can only edit app.js, you cannot touch index.html. jQuery is provided.



  • index.html contains two elements: one for the search term and one for the results.



  • The results should show as a list with each item in an "owner/name" format.



  • When a result is clicked, display an alert with the repo's language, followers, url and description.



  • The search term should be cached so duplicate searches do not trigger further requests.



  • Solution does not need to support older browsers.




Here are the app.js I submitted and the index.html.



`(function(window){
// Setup the variables for the application
var search,
results,
textFieldValue,
requestBaseUrl = 'https://api.github.com/legacy/repos/search/';

// Enable browser caching for the application
$.ajaxSetup({ cache: true});

function applyPageStyling(){
$( "body { background-image:url(http://n2.9cloud.us/766/a_beautiful_large_scale_shot_for_th_100120255.jpg); font-size:20px; font-family:'Arial, Helvetica', sans-serif;}" ).appendTo( "head" );
setTimeout(function(){alertInstructions();},1000);
};

// Alert the user what to do
function alertInstructions(){
alert("Please type the term to search GitHub for in the text field and hit the ENTER key.");
};

// Function to query the github API
function gitSearch(){
// Get the value from the search field
textFieldValue = $('#search').val();

// Error handling for search box
if (textFieldValue == ""){
alert("Please enter a term to search for");
}
else {
// Make the ajax call with the text field value appended to

Solution

Looks good to me. You could cache the ul:

var repoList = $("").appendTo('#results');


So you don´t have to query the dom again when appending the list items.
Also it is better to build the list items in memory and only append the complete list to the dom. This way you only trigger one reflow.

So in the $.each loop I would

repoList.append($("somerepo"))


and then outside the loop

$("#results").html(repoList);

Code Snippets

var repoList = $("<ul>").appendTo('#results');
repoList.append($("<li>somerepo</li>"))
$("#results").html(repoList);

Context

StackExchange Code Review Q#35714, answer score: 5

Revisions (0)

No revisions yet.