Compare function

[johankristiansson]

This is an alternative to #30 The changes allow for the user to implement a custom compare function. This means that specific logic can be handled outside of the API, keeping it fresh2death.

[codecov[bot]]

Codecov Report

Merging #31 into master will increase coverage by 0.06%. The diff coverage is 6.66%.

@@            Coverage Diff             @@
##           master      #31      +/-   ##
==========================================
+ Coverage   86.49%   86.55%   +0.06%     
==========================================
  Files         149      148       -1     
  Lines        1414     1413       -1     
  Branches       86       92       +6     
==========================================
  Hits         1223     1223              
+ Misses        191      190       -1

Impacted Files	Coverage Δ
src/amount.ts	41.66% <6.66%> (-2.04%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8fbc363...f82f31e. Read the comment docs.

[jonaskello]

I don't like allowNullOrUndefined but since it was there from the beginning I guess we need to have it for now an deprecate it later.

@johankristiansson Could you add some tests too and update the changelog?

[codecov[bot]]

Codecov Report

Merging #31 into master will increase coverage by 2.2%. The diff coverage is 57.69%.

@@            Coverage Diff            @@
##           master      #31     +/-   ##
=========================================
+ Coverage   86.49%   88.69%   +2.2%     
=========================================
  Files         149      148      -1     
  Lines        1414     1415      +1     
  Branches       86       95      +9     
=========================================
+ Hits         1223     1255     +32     
+ Misses        191      160     -31

Impacted Files	Coverage Δ
src/amount.ts	63.01% <57.69%> (+19.31%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8fbc363...45b0964. Read the comment docs.

[johankristiansson]

@jonaskello I agree regarding allowNullOrUndefined but I think we should open a separate issue for that. However, I've added tests and updated the changelog.

[jonaskello]

I inspected the original code and found that allowNullOrUndefined is only set to true in equals and compareTo. The exported API does not expose allowNullOrUndefined.

Perhaps we can build the allowNullOrUndefined handling into the equals and compareTo functions instead of exposing it?

This is the original code:

export function equals<T1 extends Quantity, T2 extends T1>(
  left: Amount<T1>,
  right: Amount<T2>
): boolean {
  return _comparison(left, right, true) === 0;
}

export function compareTo<T1 extends Quantity, T2 extends T1>(
  left: Amount<T1>,
  right: Amount<T2>
): number {
  return _comparison(left, right, true);
}

The new code would be something like:

export function equals<T1 extends Quantity, T2 extends T1>(
  left: Amount<T1>,
  right: Amount<T2>
): boolean {
  // Add whatever logic allowNullOrUndefined=true had here
  const n = compareNulls(left, right)
  if(n) return n;

  return _comparison(left, right) === 0;
}

export function compareTo<T1 extends Quantity, T2 extends T1>(
  left: Amount<T1>,
  right: Amount<T2>
): number {
  // Add whatever logic allowNullOrUndefined=true had here
  const n = compareNulls(left, right)
  if(n) return n;

  return _comparison(left, right);
}

function compareNulls(left, right): number | null {
 // Handle nulls
    if (
      (left === null && right === null) ||
      (left === undefined && right === undefined)
    ) {
      return 0;
    }
    if (left === null || left === undefined) {
      return 1;
    }
    if (right === null || right === undefined) {
      return 2;
    }
return null;
}

[jonaskello]

I think the above suggestion would mean that the allowNullOrUndefined param could be removed from the compare function as it is now handled one level up in equals and compareTo. The other calls had allowNullOrUndefined=false so for them no handling is required.

[johankristiansson]

You are correct - allowNullOrUndefined only exists in a mid tier in the API. No need to expose it to the outside world. I've updated the PR to match this.

Also added comparer to the functions min, max and clamp since those use compare functions internally.

[jonaskello]

Looks good now :-).

[jonaskello]

I'll merge it and you can go ahead and release it to npm.

[jonaskello]

Or perhaps it is better you merge when you are ready :-).