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.
Please post your comments with:
Looks good
Looks good, but here is how I would do it.
Looks bad, but here is how I would do 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
anddescription
.- The search term should be cached so duplicate searches do not trigger further requests.
- Solution does not need to support older browsers.
app.js content I've submitted:
(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(){
$( "<style>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;}</style>" ).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 the url. The callback will
// output the results or alert the error message
$("<ul>").appendTo('#results');
$.getJSON(requestBaseUrl+textFieldValue, function(json, status){})
// Success then append items to #results div
.done(function(json){
$.each(json.repositories, function(i, repositories){
$("<li>"+json.repositories[i].owner+" / "+json.repositories[i].name+"</li>")
.appendTo('#results ul')
// Add a little styling, just a little.
.css({'cursor': 'pointer','list-style-type':'none', 'width':'500px'})
.hover(
function(event) {
$(this).css({'color':'#ffffff'});
},
function(event) {
$(this).css({'color':'#000000'});
}
)
.click(function(event){
alert("Repo Lanuage is : "+json.repositories[i].language +"\n"+
"There are "+ json.repositories[i].followers +" followers of this repo.\n"+
"The URL of the repo is : "+ json.repositories[i].url +"\n"+
"The description of the repo : "+ json.repositories[i].description);
});
});
})
// Log the error message
.fail(function(status){
console.log(status);
});
}
};
// Enter key event handler
$(document).keypress(function(e) {
if(e.which == 13) {
gitSearch();
}
});
applyPageStyling();
}(window));
index.html
<!DOCTYPE html>
<html lang="en">
<head>
<meta http-equiv="Content-type" content="text/html; charset=utf-8">
<title>Search</title>
<script src="http://ajax.googleapis.com/ajax/libs/jquery/1/jquery.min.js"></script>
<script src="app.js"></script>
</head>
<body>
<input id="search" type="text"/>
<div id="results"></div>
</body>
</html>
$.ajaxSetup({ cache: true});
is reiterating adefault
setting. – SpYk3HH Nov 22 '13 at 15:01