Modulus calculation is not correct.

[giorgiofran]

There is a problem with modulus calculation when the dividend and/or the divisor are negative numbers.

I attach a simple test file with a new method mod that could be a possible solution (useful to understand the logic) and a more compact one (mod2).

import 'package:rational/rational.dart';
import 'package:test/test.dart';

Rational mod(Rational one, Rational other) {
  Rational remainder = one.remainder(other);
  if (remainder == new Rational(0)) return remainder;
  if (remainder.isNegative) {
    if (other.isNegative) {
      return remainder;
    }
    return other + remainder;
  }
  if (other.isNegative) return other + remainder;
  return remainder;
}

Rational mod2(Rational one, Rational other) {
  Rational remainder = one.remainder(other);
  if (remainder.signum + other.signum == 0) return other + remainder;
  return remainder;
}

main() {

  test('test Modulus', () {
    expect(new Rational(5) % new Rational(4), equals(new Rational(1)));
    expect(new Rational(-5) % new Rational(4), equals(new Rational(3)));
    expect(new Rational(5) % new Rational(-4), equals(new Rational(-3)));
    expect(new Rational(-5) % new Rational(-4), equals(new Rational(-1)));
    expect(new Rational(4) % new Rational(4), equals(new Rational(0)));
    expect(new Rational(-4) % new Rational(4), equals(new Rational(0)));
    expect(new Rational(4) % new Rational(-4), equals(new Rational(0)));
    expect(new Rational(-4) % new Rational(-4), equals(new Rational(0)));
  });

  test('test new version', () {
    expect(mod(new Rational(5), new Rational(4)), equals(new Rational(1)));
    expect(mod(new Rational(-5), new Rational(4)), equals(new Rational(3)));
    expect(mod(new Rational(5), new Rational(-4)), equals(new Rational(-3)));
    expect(mod(new Rational(-5), new Rational(-4)), equals(new Rational(-1)));
    expect(mod(new Rational(4), new Rational(4)), equals(new Rational(0)));
    expect(mod(new Rational(-4), new Rational(4)), equals(new Rational(0)));
    expect(mod(new Rational(4), new Rational(-4)), equals(new Rational(0)));
    expect(mod(new Rational(-4), new Rational(-4)), equals(new Rational(0)));
  });

  test('test new Version "compact"', () {
    expect(mod2(new Rational(5), new Rational(4)), equals(new Rational(1)));
    expect(mod2(new Rational(-5), new Rational(4)), equals(new Rational(3)));
    expect(mod2(new Rational(5), new Rational(-4)), equals(new Rational(-3)));
    expect(mod2(new Rational(-5), new Rational(-4)), equals(new Rational(-1)));
    expect(mod2(new Rational(4), new Rational(4)), equals(new Rational(0)));
    expect(mod2(new Rational(-4), new Rational(4)), equals(new Rational(0)));
    expect(mod2(new Rational(4), new Rational(-4)), equals(new Rational(0)));
    expect(mod2(new Rational(-4), new Rational(-4)), equals(new Rational(0)));
  });

}

[a14n]

I try to provide the same result as Dart VM.

main() {
  print(5 % 4);                                 // 1
  print((-5) % 4);                              // 3
  print(5 % (-4));                              // 1
  print((-5) % (-4));                           // 3
  print(4 % 4);                                 // 0
  print((-4) % 4);                              // 0
  print(4 % (-4));                              // 0
  print((-4) % (-4));                           // 0
  print(new Rational(5) % new Rational(4));     // 1
  print(new Rational(-5) % new Rational(4));    // 3
  print(new Rational(5) % new Rational(-4));    // 1
  print(new Rational(-5) % new Rational(-4));   // 3
  print(new Rational(4) % new Rational(4));     // 0
  print(new Rational(-4) % new Rational(4));    // 4 - incorrect
  print(new Rational(4) % new Rational(-4));    // 0
  print(new Rational(-4) % new Rational(-4));   // 4 - incorrect
}

There are indeed 2 incorrect results but not those you mentioned.

[giorgiofran]

Also some tests in rational package were wrong. The following are all the tests, some were correct.

  expect(mod(new Rational(2), new Rational(1)), equals(new Rational(0)));
    expect(mod(new Rational(0), new Rational(1)), equals(new Rational(0)));
    expect(mod(new Rational(89, 10), new Rational(11, 10)), equals(new Rational(1, 10)));
    expect(mod(new Rational(-12, 10), new Rational(5, 10)), equals(new Rational(3, 10)));
    expect(mod(new Rational(-12, 10), new Rational(-5, 10)), equals(new Rational(-2, 10)));

[giorgiofran]

Not all the cases in my original post were wrong. As far as I Know the correct results should be: 5 mod 4 = 1 -5 mod 4 = 3 5 mod -4 = -3 -5 mod -4 = -1 4 mod 4 = 0 -4 mod 4 = 0 4 mod -4 = 0 -4 mod -4 = 0

[giorgiofran]

Effectively, the VM results are wrong (at least when the result should be negative) ... , they are different also from Javascript (also these are wrong :-( ). I gave a look on wikipedia and every language has its own way of calculating modulus... Anyway, I believe that the right one is the one I suggested you.

[a14n]

As I said above

I try to provide the same result as Dart VM

On the VM:

  print(5 % (-4));                              // 1
  print((-5) % (-4));                           // 3

So I can not change these results unless the VM results are changed.

[zoechi]

Might be related to https://github.com/dart-lang/sdk/issues/28408#issuecomment-273212205

[a14n]

See also dart-lang/sdk#493

[giorgiofran]

Based on wikipedia, both the calculation I proposed and the VM one are correct. There is not an unique way of calculating the modulus. So, the only problem to be fixed is when the remainder is zero.

[a14n]

Fixed in version 0.1.10.

[a14n]

humm, still an issue in browser... :-/

[a14n]

fix in browser too in 0.1.10+1