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.
1 Answer 1
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 asisNaN(value)
JavaScript will coerce a string to a number if you use the multiply operator
*
on it. No need to useparseFloat
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