Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`modulo` is actually `remainder` #186

Closed
wavebeem opened this issue Jul 21, 2014 · 13 comments
Closed

`modulo` is actually `remainder` #186

wavebeem opened this issue Jul 21, 2014 · 13 comments

Comments

@wavebeem
Copy link
Contributor

@wavebeem wavebeem commented Jul 21, 2014

Per this Stack Overflow question, the % operator is actually the remainder operation, not modulus (same in JS as C).

So strictly speaking it ought to be renamed in Ramda, though I think providing an alias would be a decent compromise since people will probably go looking for "mod" or similar when they pick up the library.

@davidchambers
Copy link
Member

@davidchambers davidchambers commented Jul 21, 2014

I don't think an alias is the way to go. Modulus and remainder are different functions, as you say, so should have different definitions.

@CrossEye
Copy link
Member

@CrossEye CrossEye commented Jul 21, 2014

I agree that they should be different functions. And I'm pulled in two directions. The mathematician part of me (and that's my formal education) really thinks that since mathematically,

-3 mod 4 ≅ 1

then

mod(-3, 4); //=> 1

and we can simply use

remainder(-3 , 4); ==> -3

But the I've-been-coding-in-these-C-style-languages-for-years part thinks that since

-3 % 4; //=> -3

then

mod(-3, 4); //=> -3

and we need another name. I can't even come up with something better than the ugly:

modMath(-3, 4); //=> 1

The fact that I can't come up with a good name swings me a bit towards the first version, but then it would have to be documented thoroughly, as people pronouce the % operator as "mod" or "modulus", even if it really is a "remainder" operator.

@buzzdecafe
Copy link
Member

@buzzdecafe buzzdecafe commented Jul 21, 2014

function realMod(x, p) { return x < 0 ? ((x % p) + p) % p : x % p; }
realMod(-17, 5) // 3
-17 % 5 // 2
realMod(17, 0) // NaN
17 % 0 // NaN
@megawac
Copy link
Contributor

@megawac megawac commented Jul 21, 2014

What if p is negative there?

Maybe

function realMod(x, p) {
     return ((x % p) + Math.abs(p)) % p;
}
@buzzdecafe
Copy link
Member

@buzzdecafe buzzdecafe commented Jul 21, 2014

i think negative p would work like native %

@wavebeem
Copy link
Contributor Author

@wavebeem wavebeem commented Jul 21, 2014

people pronouce the % operator as "mod" or "modulus", even if it really is a "remainder" operator

And this was exactly my point in suggesting to make them just an alias. C has ruined "mod" for programmers. We could call % mod and remainder and add a true modulo as modulo. Or we could just leave out real modulo. I mean Ramda doesn't have integer division as an operator either.

What about commonResidue? 😂 (jk)

@davidchambers
Copy link
Member

@davidchambers davidchambers commented Jul 21, 2014

If nucleotides are added to Ramda, the Ramda equivalent of nucleotides.operator.binary['%'] would provide the semantics of JavaScript's % operator. This wouldn't prevent people from assuming that Ramda.mod behaves the same way, but I don't see how we could prevent that.

@buzzdecafe
Copy link
Member

@buzzdecafe buzzdecafe commented Jul 24, 2014

this one sucks, because the term is mapped to the wrong operation by the language. I don't think we should try to overcome that. So that means keeping modulo (and possibly alias mod and remainder) mapped to the % operator.

But a real modulo operation is desirable too. So again, we're down to a naming problem. Candidates so far include:

  • modMath
  • mathMod
  • mmod
  • modm
  • realMod
  • notStupidMod
  • notRemainder
  • slapAndTickle

open to other candidates, of course. I'm inclined toward mmod or mathMod but would not fight for either one. But I'd like to come to a decision on this and remove this from the issue queue.

@CrossEye
Copy link
Member

@CrossEye CrossEye commented Jul 24, 2014

👍 for mathMod.

Something like this?

var mathMod = function(n, m) {
    if (!isInteger(m) || m < 1) {return NaN;}
    if (!isInteger(n)) {return NaN;}
    return  n - (m * Math.floor(n / m));
} 

We'd still need to add an (internal?) isInteger function. I think the math version should be stricter, meaning that it only works for integers, and only when the modulus is positive. Everything else wouldn't throw an error, just return NaN. Does this make sense?

@buzzdecafe
Copy link
Member

@buzzdecafe buzzdecafe commented Jul 24, 2014

i don't think u need those type-checks, it will return NaN anyways

var mathMod = function(n, m) {
    return  n - (m * Math.floor(n / m));
} 
mathMod('w', 7) // NaN
mathMod(17, 0) // NaN
mathMod(17, -5) // -3 ok this is different
mathMod(17, 5) // 2
mathMod(-17, 5) // 3
@CrossEye
Copy link
Member

@CrossEye CrossEye commented Jul 24, 2014

As we discussed offline, I think we should also have this for the mathematical one:

mathMod(17.2, 5); //=> NaN
mathMod(17, 3.141592); //=> NaN
@CrossEye
Copy link
Member

@CrossEye CrossEye commented Jul 24, 2014

But I really don't like the type-checks in that code... ugly!

@buzzdecafe
Copy link
Member

@buzzdecafe buzzdecafe commented Jul 25, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.