3
\$\begingroup\$
const express = require('express');

const app = express();
const PORT = process.env.PORT || 3000;

const validateParams = (req, res, next) => {
  const { celsius } = req.query;

  if (!celsius) {
    return res.status(400).json({ error: 'Please provide Celsius temperature' });
  }

  const celsiusFloat = parseFloat(celsius);
  if (isNaN(celsiusFloat)) {
    return res.status(400).json({ error: 'Invalid temperature format. Please provide a number' });
  }

  next();
};

app.get('/convert', validateParams, (req, res) => {
  const { celsius } = req.query;
  const fahrenheit = (parseFloat(celsius) * 9 / 5) + 32;
  res.json({ celsius: celsius, fahrenheit: fahrenheit });
});

app.listen(PORT, () => {
  console.log(`Server is running on port ${PORT}`);
});

It's a simple API, but I am wondering if there's anything wrong, and what could be improved. I am trying to improve it as much as I can to get an idea of how to properly code.

\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Simplify

Do only what is needed. Less code is better. I don't mean code golf, good naming and layout is very important.

No magic numbers

Create constants to hold magic strings and numbers

Only if needed

Avoid needless processing. Imagine that your API is serving millions a minute. You want as little processing and bandwidth use as possible. CPU cycles cost money.

Examples

  • The call to json(obj) does not need the conversion to json. Send as text already formatted as JSON.

  • You don't need to convert to float to know a value is not a number. const valFloat = parseFloat(value); isNaN(valFloat) is the same as isNaN(value)

  • JavaScript will coerce a string to a number if you use the multiply operator * on it. No need to use parseFloat and the create a variable to hold it.

Rewrite

Keep your logs clear. I added a constant flag const LOG = false that will block output to the console unless true. Only set to true when debugging.

Important messages will get lost if your console is full of needless status messages.

const app = require("express")();
const DEFAULT_PORT      = 3000;
const BAD_REQUEST       = 400;
const NO_VALUE_ERROR    = `{ "error": "Provide temperature." }`;
const BAD_FORMAT_ERROR  = `{ "error": "Invalid temperature format." }`;
const API_PATH          = `/convert`;
const API_DESCRIPTION   = `celsius to fahrenheit,`;
const PORT              = process.env.PORT || DEFAULT_PORT;
const LOG               = false;

const reqError = (res, error) => res.status(BAD_REQUEST).send(error);
const validate = (req, res, valid) => {
    if (!req.query.celsius) { return reqError(res, NO_VALUE_ERROR); }
    if (isNaN(req.query.celsius)) { return reqError(res, BAD_FORMAT_ERROR); }
    valid();
};

app.get(API_PATH, validate, (req, res) => {
    res.json({
        celsius: req.query.celsius, 
        fahrenheit: (req.query.celsius * 9 / 5) + 32
    });
});

app.listen(PORT, () => { 
    LOG && console.log(`Started ${API_PATH} ${API_DESCRIPTION} port ${PORT}`);
});

Too cold?

There is a minimum possible temperature, absolute zero at -273.15c (-459.67f) converting values below that makes no sense?

Also note that the strings "Infinity" and "-Infinity" will both convert to Number where isNaN("Infinity") === false and isNaN(parseFloat("Infinity")) === false

\$\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.