A UNIX timestamp microservice

Question

I have made a small timestamp microservice that converts from UNIX time to a natural language date, and vice versa. Please let me know of anything I could do better.

Here is a demo. Here is the time.js file that holds the class that controls converting the time:

let debug = require('debug')('test');
let moment = require('moment');


class TimeConverter {
  constructor(){
    this.naturalDate = null;
    this.unixTime = null;
  }
  //check if string meets a specific format
  checkString(string){
    let format = /\w{3,9}?\s\d{1,2}?,\s\d{4}?/;
    let unixFormat = /^\d[0-9]{0,20}$/;
    let test = format.test(string);
    let test2 = unixFormat.test(string);

    if (test === true){
      this.natToUnix(string);

    } else if (test2 === true) {
      this.unixToNat(string);

    } else {
      this.naturalDate = null;
      this.unixTime = null;
    }

    return this.getResults();

  }
  //convert natural language date to unix
  natToUnix(string){
    //put into iso format for moment.js
    let newDate = new Date(string);
    //change into unix time
    let date = moment(newDate).unix();
    this.unixTime = date;
    this.naturalDate = string;


  }
  //change unix time to natural language date
  unixToNat(string){

    let date = moment.unix(string);
    let formatted = date.format("MMMM DD YYYY");

    this.unixTime = string;
    this.naturalDate = formatted;


  }

  getResults(){
    let results = {unix: this.unixTime, natural: this.naturalDate};
    debug(results);
    return results;
  }



}

let time = new TimeConverter();

module.exports = time;

Here is the server.js file that has some express code. It handles the routes in the application. It grabs the input from the request parameters, and serves the CSS / index.html files:

  let express = require('express');
  let debug = require('debug')('test');

  let app = express();

  let bodyParser = require('body-parser');
  let time = require('./time.js');
  let port = process.env.PORT || 8080;

  app.use(bodyParser.json());
  //serves static files
  app.use('/static', express.static('public'));
  app.use(bodyParser.urlencoded({ extended: true}));
  //route for inputting the time
  app.get(['/:time','/'], function(req,res){
    res.sendFile(__dirname + "/index.html");
    let data = req.params;

    if (data.time != undefined){
        res.json(time.checkString(data.time));
    }

  });

  //tell server to listen on a port
  app.listen(port, function(){
    debug("listening on " + port);
  });

Here is the repo.

Answer 1

Design

It's not great that some functions that manipulate internal state are exposed. It would be better to make them private, inaccessible from outside the class.

Alternatively, change them in a way that they will not modify the internal state.

Avoid unnecessary evaluation

Here, test2 doesn't need to be evaluated when test is true:

let format = /\w{3,9}?\s\d{1,2}?,\s\d{4}?/;
let unixFormat = /^\d[0-9]{0,20}$/;
let test = format.test(string);
let test2 = unixFormat.test(string);

if (test === true){
  this.natToUnix(string);

} else if (test2 === true) {
  this.unixToNat(string);

I would rewrite like this:

if (this.isNaturalFormat(input)) {
  return this.getResultWithUnixFormat(this.naturalToUnix(input));
}
if (this.isUnixFormat(input)) {
  return this.getResultWithNaturalFormat(this.unixToNatural(input));
}
return this.getResult();

All the functions I invented here, I suggest to write them ;-) That is, getResultWithUnixFormat takes as parameter a date in unix format, and returns {unix: ?}, and so on.

Use boolean values directly

Since .test(...) returns a boolean, you could use if (test) instead of this:

let test = format.test(string);
// ...

if (test === true){
  // ...

Naming

test and test2 don't tell much about their purpose:

let format = /\w{3,9}?\s\d{1,2}?,\s\d{4}?/;
let unixFormat = /^\d[0-9]{0,20}$/;
let test = format.test(string);
let test2 = unixFormat.test(string);

I suggest to rename them to isNaturalFormat and isUnixFormat.

Also, I would spell out nat as natural in unixToNat and natToUnix.

Thread-safety

It's true that thread-safety is not a practical concern in JavaScript, as user code is typically executed on a single thread in browsers and in node.js too. However, I think it's still a good idea to write thread-safe code when it's easily possible, in order to avoid picking up bad habits that may bite you later in other languages.

The implementation is not thread-safe. checkstring calls functions that mutate the internal state of the instance (naturalDate and unixTime), so if this function gets called from concurrent threads, you may get incorrect results.

You can easily make it thread-safe by eliminating the naturalDate and unixTime fields. You don't need them. These can be local variables in checkstring, and passed as parameters to getResults.

In fact, the best would be to replace getResults with dedicated functions that return {unix: ?, natural: ?} with the appropriate value filled, as I did in the examples above.