Simple static JSON - get only - API

Question

This API's aim is to return color-names for a bunch of hex colors.

Its the first time I am writing an API. And while I write lots of frontend code I rarely touched node.

I am interested in a general code review and also a API design review. I want to keep it as simple as possible: Someone with limited tech (frontend or backend) knowhow should be able to use this.

I also wanted to avoid using Express or any other framework, because I wanted to learn.

My next step would be to make it possible to look for names as well. Basically "black" would contain all the color names that contain black. (Maybe I would even register a few aliases black => night => dark)

I am looking forward to your review.

const http = require('http');
const url = require('url');
const fs = require('fs');
const nearestColor = require('../node_modules/nearest-color/nearestColor.js');
const colors = JSON.parse(
  fs.readFileSync(__dirname + '/../dist/colornames.json', 'utf8')
);
const port = process.env.PORT || 8080;
const currentVersion = 'v1';
const APIurl = ''; // subfolder for the API
const baseUrl = `${APIurl}${currentVersion}/`;

/**
 * disassembles a HEX color to its RGB components
 * @param   {string} hex hex color representatin
 * @return  {object}     {r,g,b}
 */
const hexToRgb = (hex) => {
  const int = parseInt(hex.replace('#', ''), 16);
  return {
    r: (int >> 16) & 255,
    g: (int >> 8) & 255,
    b: int & 255
  };
};

// object containing the name:hex pairs for nearestColor()
const colorsObj = {};

colors.forEach((c) => {
  // populates object needed for nearestColor()
  colorsObj[c.name] = c.hex;
  // transform hex to RGB
  c.rgb = hexToRgb(c.hex);
});

const nc = nearestColor.from(colorsObj);
/**
 * validates a hex color
 * @param   {string} color hex representation of color
 * @return {boolen}
 */
const validateColor = (color) => (
  /(^[0-9A-F]{6}$)|(^[0-9A-F]{3}$)/i.test(color)
);

/**
 * names an array of colors
 * @param   {array} colorArr array containing hex values without the hash
 * @return  {object}         object containing all nearest colors
 */
const nameColors = (colorArr) => {
  let colors = {};
  colorArr.forEach((hex) => {
    const closestColor = nc(`#${hex}`);
    const rgb = hexToRgb(hex);
    colors[`#${hex}`] = {
      name: closestColor.name,
      hex: closestColor.value,
      rgb: closestColor.rgb,
      isExactMatch: closestColor.rgb.r === rgb.r &&
                    closestColor.rgb.g === rgb.g &&
                    closestColor.rgb.b === rgb.b
    };
  });
  return colors;
};

const httpRespond = (response, responseObj = {}, statusCode = 200) => {
  response.writeHead(statusCode, {
    'Access-Control-Allow-Origin': '*',
    'Access-Control-Allow-Methods': 'POST, GET, PUT, DELETE, OPTIONS',
    'Access-Control-Allow-Credentials': false,
    'Access-Control-Max-Age': '86400',
    'Access-Control-Allow-Headers': 'X-Requested-With, X-HTTP-Method-Override, Content-Type, Accept',
    'Content-Type': 'application/json; charset=utf-8',
  });
  // ends the response with the API answer
  response.end(JSON.stringify(responseObj));
};

const requestHandler = (request, response) => {
  const requestUrl = url.parse(request.url);
  const isAPI = requestUrl.pathname.indexOf(baseUrl) !== -1;
  let statusCode = 404;
  let colorQuery = request.url.toLowerCase();
      colorQuery = colorQuery.split(baseUrl)[1] || '';

  const urlColorList = colorQuery.split(',');
  const responseObj = {status: 'Someting went wrong', colors: {}};
  const invalidColors = urlColorList.filter((hex) => (
    !validateColor(hex) && hex
  ));

  if (!isAPI) {
    responseObj.status = 'invalid URL: make sure to provide the API version';
    statusCode = 404;
  } else if (!urlColorList[0]) {
    responseObj.status = `no color(s) provided, returning all the ${colors.length} colors`;
    responseObj.colors = colors;
    statusCode = 200;
  } else if (invalidColors.length) {
    responseObj.status = `'${invalidColors.join(', ')}' is not a valid HEX color`;
    statusCode = 404;
  } else if (!invalidColors.length && isAPI) {
    responseObj.status = `names for '${urlColorList.join(', ')}' returned`;
    responseObj.colors = nameColors(urlColorList);
    statusCode = 200;
  }

  httpRespond(response, responseObj, statusCode);
};

const server = http.createServer(requestHandler);
server.listen(port, '0.0.0.0', (error) => {
  if (error) {
    return console.log(`something terrible happened: ${error}`);
  }
  console.log(`Server running and listening on port ${port}`);
  console.log(`http://localhost:${port}/${baseUrl}`);
});

This state of the code can also be viewed here: https://github.com/meodai/color-names/blob/b7f7da7db5ce42b400bb08f6f00ab43d42bb67f1/scripts/server.js

Bonus question: Would if be worth it to work with streams? If yes how would you implement that

Answer 1

requestHandler

I find the implementation requestHandler hard to read and understand. The first half of the function populates various objects, and the second half acts on the various combinations of those values. It would be easier to read if you use some early returns, and that way is more likely to avoid some unnecessary processing too.

I believe this is equivalent, but a bit easier to read, and with some impossible execution paths eliminated:

const notFound = (response, msg) => httpRespond(response, {status: msg}, 404);

const requestHandler = (request, response) => {
  const path = url.parse(request.url).pathname;
  if (path.indexOf(baseUrl) !== -1) {
    notFound(response, 'invalid URL: make sure to provide the API version');
    return;
  }

  let colorQuery = (path.split(baseUrl)[1] || '').toLowerCase();
  if (!colorQuery) {
    var responseObj = {
      status: `no color(s) provided, returning all the ${colors.length} colors`,
      colors: colors
    };
    httpRespond(response, responseObj);
    return;
  }

  const urlColorList = colorQuery.split(',');
  const invalidColors = urlColorList.filter(hex => !validateColor(hex) && hex);

  if (invalidColors.length) {
    notFound(`'${invalidColors.join(', ')}' is not a valid HEX color`);
    return;
  }

  var responseObj = {
    status: `names for '${urlColorList.join(', ')}' returned`,
    colors: nameColors(urlColorList)
  }
  httpRespond(response, responseObj);
};

API design

As mentioned earlier, I found it difficult to understand the supported URL request schemes. Some comments would have been helpful, explaining the supported patterns with examples.

The responseObj in 404 responses include empty colors property. That's unnecessary, and inappropriate, as users should not try to access this property in case of failures.

The httpRespond function returns too permissive headers. Most notably, your service does not really allow all the POST, GET, PUT, DELETE, OPTIONS methods. It seems to me that just GET would be enough. I think it's a good policy to try to be as minimalistic as possible.

Validation logic of invalid colors

The implementation filters the input colors to get a list of invalid colors. If the list is not empty, it returns with failure. It would be better to stop iterating over the colors on the first invalid found.

Also, somewhat inconsistently, the algorithm tolerates empty color strings. I think it would make more sense to fail on such input.

Technique

In this code:

  const requestUrl = url.parse(request.url);
  const isAPI = requestUrl.pathname.indexOf(baseUrl) !== -1;
  let colorQuery = request.url.toLowerCase();
      colorQuery = colorQuery.split(baseUrl)[1] || '';

It's suspicious coding that in the assignment of isAPI, there is a search for baseUrl in the request url, but then in the assignment of colorQuery, there is a search for baseUrl in the lowercased request url. This could result in odd corner cases.

Note that some minor improvements in terms of performance are also possible:

• There's no need to lowercase the entire url. It's really just the list of colors you need to lowercase, so you could do just that.
• There's no need to split the entire url, when you already have just the path part in requestUrl.pathname, so you could just split on that.

Of course these "performance issues" are negligible, and most probably not measurable. But I think it's good to keep these things in mind as a principle.

Regex

This regex /(^[0-9A-F]{6}$)|(^[0-9A-F]{3}$)/ can be written with slightly less duplication:

/^[0-9A-F]{3}([0-9A-F]{3})?$/

Also, the color validation method uses the i flag on the regex to make it case insensitive, but the caller always lowercases the string first. The performance would be better if you drop the i flag and change the A-F ranges to a-f.