buge/ts-units

Consider providing plain data structures without methods

lo1tuma opened this issue · 4 comments

Hi 👋,

I’m looking for a way to safely type time units in my project. Unfortunately there are not much libraries out there in the wild, so I was quite exited about this one. Unfortunately I can’t use it in my project, since I can’t write unit tests with the node (v16) assert module:

const assert = require('assert');
const { seconds } = require('@buge/ts-units/time');

assert.deepStrictEqual(seconds(10), seconds(10));

throws AssertionError [ERR_ASSERTION]: Values have same structure but are not reference-equal.

Also the diff is not really nice since the objects returned by seconds() are huge with a lot of methods attached. I’m not sure if the root-cause is a problem within nodejs or within this library. But I’m also a bit concerned about those huge objects. For my use case I would need to create quite a few instances with seconds() within short time-interval so this might affect the performance (I haven’t tested it though).

Ideally seconds() would just return a data-only object, e.g. { value: 10, unit: 'seconds' } and all the methods could be moved to standalone functions, e.g.:

const { seconds, Time, plus } = require('@buge/ts-units/time');
console.log(plus(seconds(10), seconds(5));
buge commented

Hi Matthias,

Thanks a lot for your interest and your feedback.

Regarding pretty error messages

Quantities in ts-units can be pretty-printed using toString(). You can write your own comparator that prints a nicely formatted error message using something like the following:

import {Dimensions, Quantity} from '@buge/ts-units';

function assertEqual(a: Quantity<Dimensions>, b: Quantity<Dimensions>) {
  if (!a.isCloseTo(b, 1e-6)) {
    throw new Error(
      `Expected ${a.toString()} (${a
        .in(b.unit)
        .toString()}) to be close to ${b.toString()}`
    );
  }
}

Resulting in:

import {minutes, seconds} from '@buge/ts-units/time';

assertEqual(minutes(1), seconds(61));
// ^ Error: Expected 1m (60s) to be close to 61s

I agree that it would be good to have some of the included in the standard library, including some comparators for things like Chai or other testing frameworks.

Object Cost

There is no difference in construction cost between what you suggest and as implemented. When you construct new quantities, the methods implemented therein are not copied into the object but remain part of the object prototype. The JavaScript interpreter simply retains a reference to the prototype on which the functions are defined. When you inspect such an object, it will list not only its own property (which is essentially just the amount, a pointer to the unit and a pointer to the prototype) but all of its own properties and those of its prototype, which is why it may appear large.

The only thing what would be cheaper than constructing objects would be to construct primitive numbers, but it's unfortunately not possible in TypeScript to ensure typesafety on an object that erases down to a plain number at compile time (as would be, for example, in C++).

Constructing and garbage collecting objects in JavaScript is extremely fast on all modern implementations (V8). If you do indeed have inner loops that need to be optimized, I would anyway recommend that you deal only with primitive numbers in those inner loops and convert to / from the typesafe values at the interfaces. But don't do this upfront. I would definitely recommend profiling your code to see where those inner loops are and whether they are actually worth optimizing.

buge commented

I'll mark this as closed for the moment, but feel free to comment further and we can reopen the issue as appropriate.

buge commented

Looking at this again, I see that we are inlining all functions in makeQuantity. Let me change this so that we are actually using a prototype, at least for quantities.

buge commented

Changes above released with v1.2.0.