An Express route, which renders a page, and makes use of socket.io

Question

This is the code for a basic endpoint in my express app routes folder. This firstly renders a page, and then on a continuous loop, renders information about some random "Breaking Bad" character.

The idea is to have the page load, and then every two seconds, the page updates automatically with the new character info. Then two seconds later, a new character replaces the one on the screen, and so on.

To do this, I'm making use of a convenient Breaking Bad REST-ful API

const express = require("express");
const app = require("../app");
const router = express.Router();
const axios = require("axios");

const socket = require("../socketioAPI");

router.get("/", function(req, res, next) {
    runCharacterUpdateInterval();
    res.render("index", { title: "Breaking Bad Page" });
});

const runCharacterUpdateInterval = () => {
    const updateTime = 2000;
    setInterval(() => {
        updatePageWithRandomCharacter();
    }, updateTime);
};

const updatePageWithRandomCharacter = () => {
    createCharacter().then(profile => {
        socket.push("breaking bad", profile);
    });
};

const createCharacter = async () => {
    const characterId = getRandomCharacterId();
    const bio = await queryAPI("characters/" + characterId);
    const { quote } = await queryAPI("quote/random?author=" + encode(bio.name));
    const { deathCount } = await queryAPI("death-count?name=" + encode(bio.name));
    return await { bio, quote, deathCount };
};

const encode = name => {
    return name.replace(/ /g, "+");
};

const getRandomCharacterId = () => {
    const totalChar = 54;
    const initalChar = 1;
    return Math.floor(Math.random() * Math.floor(totalChar)) + initalChar;
};

const queryAPI = async endpoint => {
    const api = await axios.get(`https://www.breakingbadapi.com/api/${endpoint}`);
    if (await api.data[0]) return await api.data[0];
    else return { name: [], quote: [], deathCount: [] };
};

module.exports = router;

In the past, my code has been reviewed as "spaghetti-like", and not clean. I've been trying to fix this, and so any pointers on the style, readability, or anything code related would be greatly appreciated.

Answer 1

Conditional line not in brackets

There is an if statement in queryAPI() without brackets. This impacts readability greatly- especially if the code to be conditionally executed gets moved to the line below. It is best to always include brackets.

I haven’t tested this with your code but you could simplify the if/else to a statement that uses short-circuit evaluation with logical OR:

return api.data[0] || { name: [], quote: [], deathCount: [] };

Variable names

Some variables are misleading e.g.:

const api = await axios.get(`https://www.breakingbadapi.com/api/${endpoint}`);

The return value should be an API response - not an API. Thus a name like apiResponse or response would be more descriptive.

Constants in all capitals

For constants that shouldn’t change, most style guides for JavaScript (as well as similar languages) call for constant names to be all uppercase.

E.g.

const UPDATE_TIME = 2000;
const TOTAL_CHAR = 54;
const INITIAL_CHAR = 1;

I am presuming there was a typo in the name initalChar.

It might also be wise to define URLs in constants.

Those variables could be moved out of the functions and put near the top of the code or in an included module. That way if any of them need to be updated you wouldn’t have to hunt through the code.

Simplify interval function

The callback can just be a reference. So anonymous function in the block:

setInterval(() => {
    updatePageWithRandomCharacter();
 }, updateTime);

Can be shortened to:

setInterval(updatePageWithRandomCharacter, UPDATE_TIME);

That line could replace the call to runCharacterUpdateInterval() and then that function wouldn’t be needed anymore.

Excess await keywords

Unless I am missing something, there are more await keywords than are necessary. For example:

return await { bio, quote, deathCount };

Should not need the await keyword because it is just a plain object.

The same applies to this line:

if (await api.data[0]) return await api.data[0];

Useless call to Math.floor()

The return line of getRandomCharacterId() is:

return Math.floor(Math.random() * Math.floor(totalChar)) + initalChar;

And totalChar is assigned the value 54 which is an integer and this equivalent to the floor of 54 so that call to Math.floor() can be removed.

Arrow functions could be a single line

It is not mandatory but simple functions don’t need brackets or a return- for example:

const encode = name => {
  return name.replace(/ /g, "+");
};

Could be simplified to:

const encode = name =>  name.replace(/ /g, "+");

The same is true for this promise callback:

createCharacter().then(profile => {
    socket.push("breaking bad", profile);
});

Though if the brackets were removed then the return value from the call to socket.push() would then be returned from the callback.

Answer 2

Linting

The code reads well.

There is one convention you missed: use \s to indicate whitespace.

const encode = name => {
    return name.replace(/ /g, "+");
};

const encode = name => {
    return name.replace(/\s/g, "+");
};

And the else below is redundant.

if (await api.data[0]) return await api.data[0];
else return { name: [], quote: [], deathCount: [] };

if (await api.data[0]) return await api.data[0];
return { name: [], quote: [], deathCount: [] };