Skip to content
This repository was archived by the owner on Jan 15, 2025. It is now read-only.

Commit 848269b

Browse files
jessealamajessealama
authored
Remove ability to compare decimals as digit strings (#99)
* Remove ability to compare decimals as digit strings Make comparison by mathematical value the only way we do comparison. Comparison of digits strings, we e.g. 1.2 < 1.20, can be added later if needed. * Remove unused code * Document the change * Fix a typo in an error message
1 parent cf135fd commit 848269b

File tree

4 files changed

+58
-295
lines changed

4 files changed

+58
-295
lines changed

‎CHANGELOG.md

+6
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [12.0.0] - 2024-04-10
11+
12+
### Removed
13+
14+
- The `normalize` option for `lessThan` and `equals` has been removed. The comparison is now always done on the mathematical value.
15+
1016
## [11.2.0] - 2024-03-29
1117

1218
### Changed

‎src/decimal128.mts

+9-83
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ function handleDecimalNotation(
675675
}
676676

677677
if ("-." === withoutUnderscores) {
678-
throw new SyntaxError("Lone minus sign and period anot permitted");
678+
throw new SyntaxError("Lone minus sign and period not permitted");
679679
}
680680

681681
let isNegative = !!withoutUnderscores.match(/^-/);
@@ -895,18 +895,6 @@ const DEFAULT_TOSTRING_OPTIONS: FullySpecifiedToStringOptions = Object.freeze({
895895
normalize: true,
896896
});
897897

898-
interface CmpOptions {
899-
normalize?: boolean;
900-
}
901-
902-
interface FullySpecifiedCmpOptions {
903-
total: boolean;
904-
}
905-
906-
const DEFAULT_CMP_OPTIONS: FullySpecifiedCmpOptions = Object.freeze({
907-
total: false, // compare by numeric value (ignore trailing zeroes, treat NaN as not-a-number, for a change)
908-
});
909-
910898
function ensureFullySpecifiedConstructorOptions(
911899
options?: ConstructorOptions
912900
): FullySpecifiedConstructorOptions {
@@ -975,22 +963,6 @@ function ensureFullySpecifiedToStringOptions(
975963
return opts;
976964
}
977965

978-
function ensureFullySpecifiedCmpOptions(
979-
options?: CmpOptions
980-
): FullySpecifiedCmpOptions {
981-
let opts = { ...DEFAULT_CMP_OPTIONS };
982-
983-
if (undefined === options) {
984-
return opts;
985-
}
986-
987-
if ("boolean" === typeof options.normalize) {
988-
opts.total = options.normalize;
989-
}
990-
991-
return opts;
992-
}
993-
994966
function toRational(isNegative: boolean, sg: bigint, exp: number): Rational {
995967
if (1n === sg) {
996968
// power of ten
@@ -1190,28 +1162,10 @@ export class Decimal128 {
11901162
* + 1 otherwise.
11911163
*
11921164
* @param x
1193-
* @param opts
11941165
*/
1195-
private cmp(x: Decimal128, opts?: CmpOptions): -1 | 0 | 1 | undefined {
1196-
let options = ensureFullySpecifiedCmpOptions(opts);
1197-
1198-
if (this.isNaN) {
1199-
if (options.total) {
1200-
if (x.isNaN) {
1201-
return 0;
1202-
}
1203-
return 1;
1204-
}
1205-
1206-
return undefined;
1207-
}
1208-
1209-
if (x.isNaN) {
1210-
if (options.total) {
1211-
return -1;
1212-
}
1213-
1214-
return undefined;
1166+
private cmp(x: Decimal128): -1 | 0 | 1 {
1167+
if (this.isNaN || x.isNaN) {
1168+
throw new RangeError("NaN comparison not permitted");
12151169
}
12161170

12171171
if (!this.isFinite) {
@@ -1237,43 +1191,15 @@ export class Decimal128 {
12371191
let rationalThis = this.rat;
12381192
let rationalX = x.rat;
12391193

1240-
let ratCmp = rationalThis.cmp(rationalX);
1241-
1242-
if (ratCmp !== 0) {
1243-
return ratCmp;
1244-
}
1245-
1246-
if (!options.total) {
1247-
return 0;
1248-
}
1249-
1250-
if (this.isNegative && !x.isNegative) {
1251-
return -1;
1252-
}
1253-
1254-
if (!this.isNegative && x.isNegative) {
1255-
return 1;
1256-
}
1257-
1258-
let renderedThis = this.toString({
1259-
format: "decimal",
1260-
normalize: false,
1261-
});
1262-
let renderedX = x.toString({ format: "decimal", normalize: false });
1263-
1264-
if (renderedThis === renderedX) {
1265-
return 0;
1266-
}
1267-
1268-
return renderedThis > renderedX ? -1 : 1;
1194+
return rationalThis.cmp(rationalX);
12691195
}
12701196

1271-
lessThan(x: Decimal128, opts?: CmpOptions): boolean {
1272-
return this.cmp(x, opts) === -1;
1197+
lessThan(x: Decimal128): boolean {
1198+
return this.cmp(x) === -1;
12731199
}
12741200

1275-
equals(x: Decimal128, opts?: CmpOptions): boolean {
1276-
return this.cmp(x, opts) === 0;
1201+
equals(x: Decimal128): boolean {
1202+
return this.cmp(x) === 0;
12771203
}
12781204

12791205
abs(): Decimal128 {

‎tests/equals.test.js

+29-107
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,6 @@ describe("equals", () => {
5656
test("use mathematical equality by default", () => {
5757
expect(a.equals(b)).toStrictEqual(true);
5858
});
59-
test("take trailing zeroes into account", () => {
60-
expect(a.equals(b, { normalize: true })).toStrictEqual(false);
61-
});
6259
test("mathematically distinct", () => {
6360
expect(a.equals(c)).toStrictEqual(false);
6461
});
@@ -87,35 +84,14 @@ describe("many digits", () => {
8784
).toStrictEqual(false);
8885
});
8986
describe("NaN", () => {
90-
test("NaN equals NaN, even if total is false", () => {
91-
expect(nan.equals(nan)).toStrictEqual(false);
87+
test("NaN equals NaN throws", () => {
88+
expect(() => nan.equals(nan)).toThrow(RangeError);
9289
});
93-
test("NaN does equal NaN, with total comparison", () => {
94-
expect(
95-
nan.equals(nan, {
96-
normalize: true,
97-
})
98-
).toStrictEqual(true);
90+
test("number equals NaN throws", () => {
91+
expect(() => one.equals(nan)).toThrow(RangeError);
9992
});
100-
test("number equals NaN is false", () => {
101-
expect(one.equals(nan)).toStrictEqual(false);
102-
});
103-
test("number equals NaN fails, with total comparison", () => {
104-
expect(
105-
one.equals(nan, {
106-
normalize: true,
107-
})
108-
).toStrictEqual(false);
109-
});
110-
test("NaN equals number", () => {
111-
expect(nan.equals(one)).toStrictEqual(false);
112-
});
113-
test("NaN equals number is false, with total comparison", () => {
114-
expect(
115-
nan.equals(one, {
116-
normalize: true,
117-
})
118-
).toStrictEqual(false);
93+
test("NaN equals number throws", () => {
94+
expect(() => nan.equals(one)).toThrow(RangeError);
11995
});
12096
});
12197
describe("minus zero", () => {
@@ -169,12 +145,6 @@ describe("zero", () => {
169145
test("negative zero vs zero", () => {
170146
expect(negZero.equals(zero)).toStrictEqual(true);
171147
});
172-
test("negative zero vs zero, normalization disabled", () => {
173-
expect(negZero.equals(zero, { normalize: true })).toStrictEqual(false);
174-
});
175-
test("zero vs negative zero, normalization disabled", () => {
176-
expect(zero.equals(negZero, { normalize: true })).toStrictEqual(false);
177-
});
178148
});
179149

180150
describe("normalization", () => {
@@ -190,18 +160,6 @@ describe("normalization", () => {
190160
test("compare normalized to normalized", () => {
191161
expect(d1.equals(d3)).toStrictEqual(true);
192162
});
193-
test("compare non-normal (1)", () => {
194-
expect(d1.equals(d2, { normalize: true })).toStrictEqual(false);
195-
});
196-
test("compare non-normal (2)", () => {
197-
expect(d2.equals(d1, { normalize: true })).toStrictEqual(false);
198-
});
199-
test("compare two non-normal values", () => {
200-
expect(d2.equals(d3, { normalize: true })).toStrictEqual(false);
201-
});
202-
test("compare two non-normal values", () => {
203-
expect(d3.equals(d2, { normalize: true })).toStrictEqual(false);
204-
});
205163
});
206164

207165
describe("examples from the General Decimal Arithmetic specification", () => {
@@ -247,104 +205,68 @@ describe("examples from the General Decimal Arithmetic specification", () => {
247205
});
248206
test("example two", () => {
249207
expect(
250-
new Decimal128("-127").equals(new Decimal128("12"), {
251-
normalize: true,
252-
})
208+
new Decimal128("-127").equals(new Decimal128("12"))
253209
).toStrictEqual(false);
254210
});
255211
test("example three", () => {
256212
expect(
257-
new Decimal128("12.30").equals(new Decimal128("12.3"), {
258-
normalize: true,
259-
})
260-
).toStrictEqual(false);
213+
new Decimal128("12.30").equals(new Decimal128("12.3"))
214+
).toStrictEqual(true); // would be false if we didn't normalize
261215
});
262216
test("example four", () => {
263217
expect(
264-
new Decimal128("12.30").equals(new Decimal128("12.30"), {
265-
normalize: true,
266-
})
218+
new Decimal128("12.30").equals(new Decimal128("12.30"))
267219
).toStrictEqual(true);
268220
});
269221
test("example five", () => {
270222
expect(
271-
new Decimal128("12.3").equals(new Decimal128("12.300"), {
272-
normalize: true,
273-
})
274-
).toStrictEqual(false);
223+
new Decimal128("12.3").equals(new Decimal128("12.300"))
224+
).toStrictEqual(true); // would be false if we didn't normalize
275225
});
276226
test("example six", () => {
277-
expect(
278-
new Decimal128("12.3").equals(new Decimal128("NaN"), {
279-
normalize: true,
280-
})
281-
).toStrictEqual(false);
227+
expect(() =>
228+
new Decimal128("12.3").equals(new Decimal128("NaN"))
229+
).toThrow(RangeError); // wouldn't throw if we did a total comparison
282230
});
283231
describe("inline examples", () => {
284232
test("example one", () => {
285233
expect(
286-
new Decimal128("-Infinity").equals(new Decimal128("-127"), {
287-
normalize: true,
288-
})
234+
new Decimal128("-Infinity").equals(new Decimal128("-127"))
289235
).toStrictEqual(false);
290236
});
291237
test("example two", () => {
292238
expect(
293-
new Decimal128("-1.00").equals(new Decimal128("-1"), {
294-
normalize: true,
295-
})
296-
).toStrictEqual(false);
239+
new Decimal128("-1.00").equals(new Decimal128("-1"))
240+
).toStrictEqual(true); // would be false if we didn't normalize
297241
});
298242
test("example three", () => {
299-
expect(
300-
new Decimal128("-0.000").equals(negZero, {
301-
normalize: true,
302-
})
303-
).toStrictEqual(false);
243+
expect(new Decimal128("-0.000").equals(negZero)).toStrictEqual(
244+
true
245+
); // would be false if we didn't normalize
304246
});
305247
test("example four", () => {
306-
expect(
307-
negZero.equals(zero, {
308-
normalize: true,
309-
})
310-
).toStrictEqual(false);
248+
expect(negZero.equals(zero)).toStrictEqual(true); // would be false if we didn't normalize
311249
});
312250
test("example five", () => {
313251
expect(
314-
new Decimal128("1.2300").equals(new Decimal128("1.23"), {
315-
normalize: true,
316-
})
317-
).toStrictEqual(false);
252+
new Decimal128("1.2300").equals(new Decimal128("1.23"))
253+
).toStrictEqual(true); // would be false if we didn't normalize
318254
});
319255
test("example six", () => {
320256
expect(
321-
new Decimal128("1.23").equals(new Decimal128("1E+9"), {
322-
normalize: true,
323-
})
257+
new Decimal128("1.23").equals(new Decimal128("1E+9"))
324258
).toStrictEqual(false);
325259
});
326260
test("example seven", () => {
327261
expect(
328-
new Decimal128("1E+9").equals(new Decimal128("Infinity"), {
329-
normalize: true,
330-
})
262+
new Decimal128("1E+9").equals(new Decimal128("Infinity"))
331263
).toStrictEqual(false);
332264
});
333265
test("example eight", () => {
334-
expect(
335-
new Decimal128("Infinity").equals(new Decimal128("NaN"), {
336-
normalize: true,
337-
})
338-
).toStrictEqual(false);
266+
expect(() =>
267+
new Decimal128("Infinity").equals(new Decimal128("NaN"))
268+
).toThrow(RangeError); // wouldn't throw if we did a total comparison
339269
});
340270
});
341271
});
342272
});
343-
344-
describe("examples from a presentation at TC39 plenary", () => {
345-
test("NaN with a payload", () => {
346-
expect(
347-
new Decimal128("NaN").equals(new Decimal128("NaN123"))
348-
).toStrictEqual(false);
349-
});
350-
});

0 commit comments

Comments
 (0)