Problem
I’ve just started to write jQuery and have just written my first code using Ajax calls. I’m fetching data from the pokeapi and am using it to random generate new Pokemon.
I have refactored it as much as possible, but I really think there must be a way to abstract the Ajax call into a function and only use one whilst still retaining the random values. I tried to do it myself but came across problems when trying to use data.name
, data.types
etc as I kept being told that data
does not exist when I abstracted it.
// There are 778 pokemon on the database
// This function allows us to generate a random number between two limits
function randomIntFromInterval(min, max) {
return Math.floor(Math.random() * (max - min + 1) + min);
};
// A more specific number between 0 and the number of poke on database
function randPokemon() {
return randomIntFromInterval(0, 718).toString();
}
// Fetch a random pokemon name
function generateName(urlinput, id) {
var generateurl = "http://pokeapi.co/api/v1/" + urlinput + randPokemon();
$.ajax({
type: "GET",
url: generateurl,
// Set the data to fetch as jsonp to avoid same-origin policy
dataType: "jsonp",
async: true,
success: function(data) {
// If the ajax call is successfull, add the name to the "name" span
$(id).text(data.name);
}
});
}
// Fetch random pokemon types
function generateTypes(urlinput, id) {
var generateurl = "http://pokeapi.co/api/v1/" + urlinput + randPokemon()
$.ajax({
type: "GET",
url: generateurl,
dataType: "jsonp",
async: true,
success: function(data) {
var types = "";
// Loop over all the types contained in an array
for (var i = 0; i < data.types.length; i++) {
// Set the current type we will add to the "types" span
var typetoAdd = (data.types[i].name);
// Capitalise the first letter of the current ability
typetoAdd = typetoAdd.charAt(0).toUpperCase() + typetoAdd.slice(1, (typetoAdd.length));
// Append the current type to the overall "types" variable
types += typetoAdd + " ";
}
// Insert each type the pokemon is into the "types" span
$(id).text(types);
}
});
}
// Fetch random pokemon abilities
function generateAbilities(urlinput, id) {
var generateurl = "http://pokeapi.co/api/v1/" + urlinput + randPokemon()
$.ajax({
type: "GET",
url: generateurl,
dataType: "jsonp",
async: true,
success: function(data) {
var abilities = "";
// Loop over all the abilities
for (var i = 0; i < data.abilities.length; i++) {
// Set the current ability we will add to the "abilities" span
var abilityToAdd = (data.abilities[i].name);
// Capitalise the first letter of the current ability
abilityToAdd = abilityToAdd.charAt(0).toUpperCase() + abilityToAdd.slice(1, (abilityToAdd.length));
// Append the current ability to the overall "abilities" variable
abilities += "<li>" + abilityToAdd + "</li>";
}
// Insert abilities to "abilities" span
$(id).html(abilities);
}
});
}
// Fetch a random pokemon image
function generateSprite(urlinput, id) {
var generateurl = "http://pokeapi.co/api/v1/" + urlinput + randPokemon()
$.ajax({
type: "GET",
url: generateurl,
dataType: "jsonp",
async: true,
success: function(data) {
var href = "http://pokeapi.co" + data.image;
// Add random image source to the sprite image source
$(id).attr("src", href);
}
});
}
// Use all generate functions together to make a new random pokemon!
function makeAPokemon() {
generateName("pokemon/", "#name");
generateTypes("pokemon/", "#types");
generateAbilities("pokemon/", "#abilities")
generateSprite("sprite/", "#sprite")
}
// If the generate button is clicked, call the makeAPokemon() function
$("#generate").on("click", makeAPokemon);
// Call the makeAPokemon() function once initial page load
makeAPokemon();
body {
background-color: #FFFFFF;
font-family: "Open Sans", sans-serif;
width: 100%;
}
header {
width: 100%;
}
h1 {
text-align: center;
}
h2 {
font-size: 1.4em;
}
main {
max-width: 600px;
margin: 0 auto;
}
img {
display: block;
margin: 0 auto;
}
.pokemon {
background-color: #E3E3E3;
border: 4px solid #000000;
}
.name,
.species {
text-align: center;
}
.sprite {
height: 100px;
}
.abilities {
margin: 30px 25px;
height: 160px;
}
.abilities h3 {
padding-left: 60px;
padding-bottom: 5px;
border-bottom: 1px solid #999999;
}
.abilities ul {
list-style-type: none;
margin-left: 15px;
}
.abilities li {
padding: 5px;
}
button {
background-color: #EA0041;
color: #FFFFFF;
width: 200px;
height: 50px;
border: 1px solid black;
box-shadow: none;
margin: 50px auto;
display: block;
}
<!doctype html>
<html lang="en">
<head>
<title>Random Pokemon Generator</title>
<link rel="shortcut icon" type="image/ico" href="/favicon.ico" />
<!-- Styles -->
<link href='http://fonts.googleapis.com/css?family=Open+Sans' rel='stylesheet' type='text/css'>
<link href="assets/css/style.min.css" rel="stylesheet" type="text/css" media="all" />
</head>
<body>
<header>
<h1>Random Pokemon Generator</h1>
</header>
<main>
<div class="pokemon" id="pokemon">
<h2 class="name">
<span>Name: </span>
<span id="name"></span>
</h2>
<h2 class="species">
<span>Type: </span>
<span id="types"></span>
</h2>
<div class="sprite">
<img src="" id="sprite">
</div>
<div class="abilities">
<h3>Abilities</h3>
<ul id="abilities">
<li>Ability One</li>
<li>Ability Two</li>
<li>Ability Three</li>
<li>Ability Four</li>
</ul>
</div>
<button id="generate" role="button">Generate!</button>
</div>
</main>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
</body>
</html>
Solution
First, a question: If there are 778 Pokemon (as your documentation says), why does randPokemon()
return a number between zero and 718? I know nothing about pokemon, so I don’t know which one is correct. (I looked it up, though: It’s 718)
Also, bug: Your current implementation may return an ID of zero, which doesn’t match any pokemon.
Anyway, overall it’s not bad, though I do have some comments:
-
randPokemon
should probably berandomPokemonID
. Since the purpose of the entire script is to generate a random pokemon, you might think that therandPokemon
function is the main function. But it just returns a number.
Or actually, it returns a string, but that’s not necessary; it only does so because you need a string later (well, you don’t; JS will happily add a number to a URL string), but that’s not this function’s concern. -
Incidentally, while I applaud that you’ve extracted a generic “random int between min and max” function, it’s an unnecessary complication in this case. Since there are a fixed number of pokemon, starting at ID 1, and ending at ID 718, all you need is:
Math.floor(Math.random() * MAX_POKEMON_ID) + 1;
-
And you’re right: Those ajax calls should be abstracted somehow. I don’t know what you tried, but I’ve included an example below.
-
Looking at the current implementation, though, I’d call all the functions
fetch...
instead ofgenerate...
; they don’t really generate data out of thin air. -
Also, there’s no need to pass the API path to the functions, when the functions are specialized. For instance,
generateSprite
should always fetch data from/sprites
, so passing that as an argument is unnecessary. However, if you abstract the fetching, it’s of course necessary to pass the endpoint path.
In terms of abstracting things, I’d start by wrapping everything in a generateRandomPokemon
function, that produces an object containing all the data you want to display. And yes, this one I would call “generate”, since it ties together several random API calls.
It’d decouple your UI-updating code from the data itself. By simply producing a plain object, you’re free create/update UI independent of the data parsing.
function generateRandomPokemon(callback) {
// "constants"
var MAX_POKEMON_ID = 718,
BASE_URL = "http://pokeapi.co";
var fetches = [], // array to hold fetch operations
pokemon = {}; // object to hold random pokemon data
function getRandomID() {
return 1 + Math.random() * MAX_POKEMON_ID | 0; // bitwise floor() trick
}
function fetchRandom(endpoint, callback) {
var url = BASE_URL + "/api/v1/" + endpoint + "/" + getRandomID();
return $.ajax({
type: "GET",
url: url,
dataType: "jsonp",
success: callback
});
}
// fetch a random name
fetches.push(fetchRandom('pokemon', function (data) {
pokemon.name = data.name;
}));
// fetch random types
fetches.push(fetchRandom('pokemon', function (data) {
pokemon.types = data.types.map(function (type) {
return type.name;
});
}));
// fetch random abilities
fetches.push(fetchRandom('pokemon', function (data) {
pokemon.abilities = data.abilities.map(function (type) {
return type.name;
});
}));
// fetch random sprite
fetches.push(fetchRandom('sprite', function (data) {
pokemon.image = BASE_URL + data.image;
}));
// when all the fetches are done, trigger the callback with
// the pokemon object.
// If there was an error, trigger it with null (not really
// informative, but better than nothing)
$.when.apply(null, fetches)
.done(function () {
callback(pokemon);
})
.fail(function () {
callback(null);
});
}
// ---------------------
function displayRandomPokemon() {
generateButton.prop("disabled", true);
generateRandomPokemon(function (data) {
generateButton.prop("disabled", false);
if(!data) {
alert("Oops"); // an error occurred
return;
}
// output example
var properties = [];
properties.push("Name: " + data.name);
properties.push("Types: " + data.types.join(", "));
properties.push("Abilities: " + data.abilities.join(", "));
$("#output").empty().text(properties.join("n"));
$("#sprite").attr("src", data.image);
});
}
var generateButton = $("#generate");
generateButton.on("click", displayRandomPokemon);
// run on load
displayRandomPokemon();
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<img id="sprite" src="">
<pre id="output"></pre>
<button id="generate" disabled>Generate</button>