Repository: josdejong/mathjs
Pull request #460 (2015-09-06 11:17:53)

Fraction.js improvements
Hi Jos,

I just had some time to have a look over your fraction.js integration and want to suggest some minor changes/optimizations. 

This includes: 
- sign checks are only necessary on the .s attribute as it is ensured that numerator and denominator are always positive
- The usage of the .neg() function, fraction.js provides
- Simple implementation of lcm() for fractions

In the string utils, I found this snippet:
```
return (value.s * value.n) + '/' + value.d;
```
I think you chose that own concat over toFraction() as it excludes the whole part of the rational number. With the newest version of fraction.js, the whole part exclusion is optional.

So if you update fraction.js, you can make  use of toFraction(). The same is true for toLatex().

Updating fraction.js in general makes sense at the moment, as I implemented some more performance improvements, which can speed up things for some use cases.

Robert


Patch:
diff --git a/lib/function/arithmetic/lcm.js b/lib/function/arithmetic/lcm.js
index f51a4b384..f9ecf640a 100644
--- a/lib/function/arithmetic/lcm.js
+++ b/lib/function/arithmetic/lcm.js
@@ -46,7 +46,13 @@ function factory (type, config, load, typed) {
 
     'BigNumber, BigNumber': _lcmBigNumber,
 
-    // TODO: implement support for Fraction
+    'Fraction, Fraction': function (x, y) {
+
+      if (x.n === 0 && y.n === 0) {
+        return new type.Fraction(0);
+      }
+      return x.mul(y).abs().div(x.gcd(y));
+    },
 
     'Matrix, Matrix': function (x, y) {
       // result
diff --git a/lib/function/arithmetic/square.js b/lib/function/arithmetic/square.js
index dafaf1a5f..2802ebaad 100644
--- a/lib/function/arithmetic/square.js
+++ b/lib/function/arithmetic/square.js
@@ -46,7 +46,7 @@ function factory (type, config, load, typed) {
     },
 
     'Fraction': function (x) {
-      return x.clone().mul(x);
+      return x.mul(x);
     },
 
     'Array | Matrix': function (x) {
diff --git a/lib/function/arithmetic/unaryMinus.js b/lib/function/arithmetic/unaryMinus.js
index 85ab52c14..c43db25f5 100644
--- a/lib/function/arithmetic/unaryMinus.js
+++ b/lib/function/arithmetic/unaryMinus.js
@@ -42,9 +42,7 @@ function factory (type, config, load, typed) {
     },
 
     'Fraction': function (x) {
-      var tmp = x.clone();
-      tmp.s = -tmp.s;
-      return tmp;
+      return x.neg();
     },
 
     'Unit': function (x) {


Test patch:
diff --git a/test/function/arithmetic/lcm.test.js b/test/function/arithmetic/lcm.test.js
index 20060a855..d5d6c3fb1 100644
--- a/test/function/arithmetic/lcm.test.js
+++ b/test/function/arithmetic/lcm.test.js
@@ -77,6 +77,27 @@ describe('lcm', function() {
     assert.throws(function () {lcm('a', 2); }, /Cannot convert "a" to a number/);
   });
 
+  it('should find the least common multiple of fractions', function () {
+    var a = math.fraction(5,8);
+    assert.equal(lcm(a, math.fraction(3,7)).toString(), '15');
+    assert.equal(a.toString(), '0.625');
+  });
+
+  it('should find the least common multiple of mixed numbers and fractions', function () {
+    assert.deepEqual(lcm(math.fraction(12), 8), math.fraction(24));
+    assert.deepEqual(lcm(12, math.fraction(8)), math.fraction(24));
+  });
+  
+  it('should find the least common even for edge cases', function () {
+    assert.deepEqual(lcm(math.fraction(-3), math.fraction(3)), math.fraction(3));
+    assert.deepEqual(lcm(math.fraction(3), math.fraction(-3)), math.fraction(3));
+    assert.deepEqual(lcm(math.fraction(0), math.fraction(3)), math.fraction(0));
+    assert.deepEqual(lcm(math.fraction(3), math.fraction(0)), math.fraction(0));
+    assert.deepEqual(lcm(math.fraction(0), math.fraction(0)), math.fraction(0));
+    assert.deepEqual(lcm(math.fraction(200), math.fraction(333)), math.fraction(66600));
+    assert.deepEqual(lcm(math.fraction(9999), math.fraction(8888)), math.fraction(79992));
+  });
+
   it('should throw an error with units', function() {
     assert.throws(function () { lcm(math.unit('5cm'), 2); }, TypeError, 'Function lcm(unit, number) not supported');
   });