Skip to content

Commit

Permalink
Pre-reduce fraction multiplication and division to stay within safe l…
Browse files Browse the repository at this point in the history
…imits
  • Loading branch information
frostburn committed Feb 8, 2024
1 parent 1146d5a commit 7e7ea4e
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 2 deletions.
7 changes: 7 additions & 0 deletions src/__tests__/fraction.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,4 +440,11 @@ describe('Fraction', () => {
foo = foo.mul(foo);
}
});

it('multiplies large cancelling factors', () => {
const one = new Fraction('1234567890/987654321').mul(
'987654321/1234567890'
);
expect(one.equals(1)).toBe(true);
});
});
6 changes: 6 additions & 0 deletions src/__tests__/monzo.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,12 @@ describe('Fraction to monzo converter', () => {
).toBeTruthy();
});

it('converts a Pythagorean interval', () => {
const [monzo, residual] = toMonzoAndResidual('129140163/134217728', 6);
expect(monzo).toEqual([-27, 17, 0, 0, 0, 0]);
expect(residual.equals(1)).toBe(true);
});

it('leaves residual 0 for zero (vector part)', () => {
const [monzo, residual] = toMonzoAndResidual(0, 1);
expect(residual.equals(0)).toBeTruthy();
Expand Down
16 changes: 14 additions & 2 deletions src/fraction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,13 @@ export class Fraction {
**/
mul(other: FractionValue) {
const {s, n, d} = new Fraction(other);
return new Fraction(this.s * this.n * s * n, this.d * d);
// Must pre-reduce to avoid blowing the limits
const ndFactor = gcd(this.n, d);
const dnFactor = gcd(this.d, n);
return new Fraction(
this.s * (this.n / ndFactor) * s * (n / dnFactor),
(this.d / dnFactor) * (d / ndFactor)
);
}

/**
Expand All @@ -541,7 +547,13 @@ export class Fraction {
if (n === 0) {
throw new Error('Division by Zero');
}
return new Fraction(this.s * this.n * s * d, this.d * n);
// Must pre-reduce to avoid blowing the limits
const nFactor = gcd(this.n, n);
const dFactor = gcd(this.d, d);
return new Fraction(
this.s * (this.n / nFactor) * s * (d / dFactor),
(this.d / dFactor) * (n / nFactor)
);
}

/**
Expand Down

0 comments on commit 7e7ea4e

Please sign in to comment.