patternjavascriptMinor
JavaScript app to search GitHub using JSON API
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...
Here are the
`(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
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,urlanddescription.
- 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:
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
and then outside the loop
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.