First and foremost, great work! I’m sure you put a lot of time and thought into this and it really shows. Excellent.
For a small project, with limited functionality. Particularly as an experiment or Proof-of-concept, it’s probably not worth changing much. However, if this were the beginnings of an application that you intended to expand upon over time, then I would make the following changes:
-
Use more specific function and variable names.
This will make your code easier to read and will cut down on the number of comments you need.
-
Break your functions down into multiple smaller functions.
To quote Bob Martin (author of Clean Code).
“The first rule of functions is that they should be small. The second rule of functions is that they should be smaller than that”
When you find yourself adding comments to describe what different parts of a function do, this is a signal that you should be breaking up the code. For example, I would refactor the following:
function createListItem(game) {
// if no results were found, return a list item specifying as much
if (game === undefined) {
return $("<li/>", {
class: "list-group-item text-center",
text: "No games were found matching your input."
});
}
// create the list item
var listItem = $("<li/>", {
class: "list-group-item result-item",
"data-game-id": game.id,
});
// some game covers were missing from API, so had to check
// if it's missing in order to display a placeholder instead
var img;
if (game.cover) {
img = game.cover.url;
}
else {
img = "https://placeholdit.imgix.net/~text?txtsize=22&txt=Missing%20cover&w=90&h=90";
}
// create a wrapper for list content and append game data to it
var wrapper = $("<div/>", {
class: "list-content-wrapper"
});
$("<img/>", {
src: img,
alt: game.name
}).appendTo(wrapper);
wrapper.append(game.name);
// append wrapper with game data to the created list item
wrapper.appendTo(listItem);
return listItem;
}
into:
function createListItemFor(game) {
if (game === undefined) {
return createGameNotFoundListItem();
}
var listItemForGame = createGameListItem(game.id);
var gameCoverImage = getGameCoverImg(game.cover);
var wrapper = createListContentWrapper();
var wrapperWithGameData = appendGameDataToWrapper(wrapper, game.name, gameCoverImage);
var listItemWithWrapper = appendWrapperToListItem(listItemForGame, wrapperWithGameData);
return listItemWithWrapper;
}
This type of refactoring makes your code easier to read, test, and reuse.
- Assign variables for your icon classes
var expandIcon = "fa-plus-square-o"
var collapseIcon = "fa-minus-square-o"
This allows you to change the design without having to perform any big search and replace operations.
- Put your modules in separate files and use a bundler like webpack
(again, only worth it if you treat this code as the starting point for a larger project).
That’s my take, based on a quick look. Hope it’s helpful. Like I said though, overall, great work.
Feel free to check out my other advice on the Free Code Camp blog at:
https://medium.freecodecamp.com/@BillSourour