Pokemon database utilizing pokeapi

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 ";
}
// 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

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 be randomPokemonID. Since the purpose of the entire script is to generate a random pokemon, you might think that the randPokemon 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 of generate...; 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();


Generate

StackExchange Code Review Q#66650, answer score: 3