1
\$\begingroup\$

Hello I have a NodeJS which act like an interface it connects the client(mobile app) to the actual server My node JS API receives data from the client to verify it and send it to the server.

I want to know does this code looks good?
Is that how API should be written in NodeJS?
Does this code violate any of the SOLID principles?
How can I improve this code?

const axios = require("axios")
require("express-async-errors")
const bodyParser = require("body-parser")
const express = require("express")

const LOLTrackingSystem = require("../methods/onlineGamesTracking/LOLTracking")
const getUserData = require("../methods/leagueOfLegends/getUserData")
//======[Gizmo Operator Settings]=======

// userId = token

//========[Gizmo API URL]==========
apiRoute = (api) => {
  api.use(bodyParser.json())
  // api.use(bodyParser.urlencoded({ extended: false }));
  // api.use(express.json());

  api.post("/api/auth", (req, res) => {
    const clientAuth = {
      username: req.body.username,
      password: req.body.password
    }

    const loginValidationURL = `http://${process.env.HOST}/api/users/${clientAuth.username}/${clientAuth.password}/valid`

    axios
      .get(loginValidationURL, { headers: { Authorization: process.env.AUTH } })
      .then((response) => {
        let token = response.data.result.identity.userId
        response.data.result.result == 0
          ? res.json({ status: "Authenticated", result: 1, token: hash })
          : res.json({ status: "Wrong Username or Password", result: 0 })
      })
      .catch((error) => {
        console.log("error " + error)
        res.json({
          status: `Couldn't reach Arena Gaming Server. Try again later`,
          result: 404
        })
      })
  })

  api.post("/api/memberProfile", (req, res) => {
    var token = req.headers["authorization"]
    // bcrypt.compare(token, hash).then(function(res) {
    //     // res == true
    // });
    const memberProfileURL = `http://${process.env.HOST}/api/users/${encryptedId}`

    axios
      .get(memberProfileURL, { headers: { Authorization: process.env.AUTH } })
      .then((response) => {
        response.data
      })
      .catch((error) => {
        console.log("error " + error)
        res.json({
          status: `Couldn't reach Arena Gaming Server. Try again later`,
          result: 404
        })
      })
  })
  //=============================================================================
  // Gizmo Related API
  //=============================================================================
  // MemberState Get The UserId & HostId when member login or logout of Gizmo
  api.post("/api/gizmo/memberState/:userId/:host/:state", async (req, res) => {
    const member = {
      state: req.params.state,
      userId: req.params.userId,
      hostComputer: req.params.host
    }
    if (Number(member.state)) {
      console.log(
        "[Logged In] Member with userId: " +
          member.userId +
          " To HostId: " +
          member.hostComputer +
          "[Sent From GizmoServer]"
      )
      await getUserData(member.userId, async (subscribed) => {
        // Check if the user is subscribed to the tracking system or not
        //*TODO* Add a list of currently tracking members that we can see
        if (subscribed) {
          LOLTrackingSystem(member.userId) // Start tracking that member
          res.json("[Started] Member " + member.userId + " Tracking")
        } else {
          res.json(
            "[Started] Member " +
              member.userId +
              " is not subscribed to the tracking system"
          )
        }
      })
    } else {
      console.log(
        "[Logged Out] Member with userId: " +
          member.userId +
          " From HostId: " +
          member.hostComputer +
          "[Sent From GizmoServer]"
      )
    }
  })
}

module.exports = apiRoute

Thanks

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

the code has plenty of room for improvement, and yes it violates the solid principals like the Single responsibility principle "your functions are way too big and the module in itself has too many responsibilities ", to clarify there are no rules on how API should be written in NodeJS there are code practices and design patterns that help us developers write more maintainable code, so don't try to follow every single rule you will get stuck in optimizing and it really depends on your project needs. Now for the improvement here are some suggestions you can work on :

  • separate your endpoints in different files, take a look at my answer here, you can use the express router
  • move your constant variables like "loginValidationURL " in a different file like config.js then import and use it
  • don't overuse URL params use post request to get objects to your server
  • Build the application with a framework independent mindset
  • Make your functions lighter separate them by roles like this take this example on your "/api/auth endpoint" it will explain many things

// Your implimation

api.post("/api/auth", (req, res) => {
  const clientAuth = {
    username: req.body.username,
    password: req.body.password
  };

  const loginValidationURL = `http://${process.env.HOST}/api/users/${clientAuth.username}/${clientAuth.password}/valid`;

  axios
    .get(loginValidationURL, { headers: { Authorization: process.env.AUTH } })
    .then(response => {
      let token = response.data.result.identity.userId;
      response.data.result.result == 0
        ? res.json({ status: "Authenticated", result: 1, token: hash })
        : res.json({ status: "Wrong Username or Password", result: 0 });
    })
    .catch(error => {
      console.log("error " + error);
      res.json({
        status: `Couldn't reach Arena Gaming Server. Try again later`,
        result: 404
      });
    });
});

/* Api Handler
 * file path : routes/api/client.js
 * in this file i only write the client api endpoints /client/auth /client/logout /client/:id the basic crud
 * i have seperate the main job in a controller module like that the api is mucj cleanter
 */
const express = require("express");
const router = express.Router();
const { authClient } = require("../controller/clientcontroller");

router.post("/client/auth", (req, res) => {
  authClient(req, res)
});

/* controller
 * file path : routes/controller/clientcontroller.js
 * in this file i only write the client controllers for each api call
 */

const { extractClientAuth } = require("./api/client/reqHandler"); // this module will be responsible for extracting and validation req objects
const { loginValidationUrlBuilder } = require("../../config"); // this module is responsible for all your static and config objects like axios custom headers
const { apiClient } = require("./services/externalJobs"); // this for exrtenal jobs and api calls
const { responseBuilder } = require("./api/tools"); // this module for creating response objects

function authClient(req, res) {
  const clientAuth = extractClientAuth(req);
  const loginValidationURL = loginValidationUrlBuilder(clientAuth);
  apiClient
    .login(loginValidationURL)
    .then(response => res.json(responseBuilder.authSuccess(response)))
    .catch(error => {
      res.json(responseBuilder.authFailed(error));
    });
}

  • this is by no mean the best way it's just a way to structures your project take from it what you like another this I can recommend this video on youtube channel Dev Mastry he does an excellent job of explaining how to create a scalable node application using express and MongoDB https://www.youtube.com/watch?v=fy6-LSE_zjI
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.