API Implementation and Architecture

Question

I recently put together an Express/Node API that consumes and transforms an additional third party API.

And while everything is working great, I feel there may be ways to improve the architecture and implementation.

This is my current implementation:

    nodes_modules/
    routes/
    ...api.js
    ...search.js
   index.js
   .babelrc
   .config.js
   .package.json

api.js

import express from "express";
let router = express.Router();

router.use('/search', require('./search'));

module.exports = router;

search.js

import express from "express";
import axios from "axios";
import transformer from "transformer";
const router = express.Router();

router.get('/:type', (req, res) => {

    const type = req.params.type;

    axios.get('http://example.com/api/' + type)
        .then((response) => {

            let transformed = {};

            switch(type) {
                case "people":
                    transformed = transformer.search(response.data, "results[*].{ name: name }");
                    break;
                case "aircrafts":
                    transformed = transformer.search(response.data, "results[*].{ name : aircraftName }");
                    break;
                default:
                    res.status(400).send({ error : "Bad request" });
                    break;
            }

            res.send({original: response.data, transformed: transformed });
        })
        .catch( () => {
            res.status(400).send({ error : "Bad request" });
        });
});

module.exports = router;

index.js

import express from 'express';
import bodyParser from 'body-parser';
import cors from 'cors';

const app = express();
app.use(cors());
app.use(bodyParser.urlencoded({  extended: true }));
app.use(bodyParser.json());

app.use('/api', require('./routes/api'));

app.use('*', (req,res) => {
   res.status(404).send('Page Not Found');
});

app.listen(3030, function() {
   console.log('running on port 3030');
});

I appreciate any suggestions for improvement, pointing out of any flaws or useful changes that can be made to the current implementation. Thank you

Answer 1

After reading over the code snippets you included, it looks pretty simple.

The only concern I have is that if the switch statement reaches the default case, then res.status(400).send() is called , and then just after the switch statement, res.send() is called... If a 400 status is sent, then it doesn't seem appropriate to send the response data (even though transformed wouldn't be assigned in the default case.

One other improvement I see is that the 400 Bad Request response could be made consistent by moving that into a function and calling it in the two places where a 400 response is sent. So in the example below, that is defined as the function badRequest() inside the callback to router.get(), or that could be moved outside the callback and would then likely need to accept the response object as a parameter.

router.get('/:type', (req, res) => {
    function badRequest() {
        res.status(400).send({ error : "Bad API request" });
    }
    const type = req.params.type;

    axios.get('http://example.com/api/' + type)
        .then((response) => {

            let transformed = {};

            switch(type) {
                //..omitting lines for brevity
                default:
                    badRequest();
                    return; //stop the function, since 400 response sent
            }

            res.send({original: response.data, transformed: transformed });
        })
        .catch( () => {
            badRequest();
        });
});