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

Rename mod and related operators to rem #2888

Merged
merged 1 commit into from
Oct 11, 2018
Merged

Rename mod and related operators to rem #2888

merged 1 commit into from
Oct 11, 2018

Conversation

mfelsche
Copy link
Contributor

as our mod is actually calculating the remainder (results have the sign of the dividend, e.g. -5 % 2 = -1),
not the modulo (result have the sign of the divident, e.g. -5 % 2 = 1).

A proper modulo operator will be added in a later PR.

as our mod is actually calculating the remainder (results have the sign of the dividend, e.g. -5 % 2 = -1),
not the modulo (result have the sign of the divident, e.g. -5 % 2 = 1).

A proper modulo operator will be added in a later PR.
@mfelsche mfelsche added the changelog - changed Automatically add "Changed" CHANGELOG entry on merge label Sep 28, 2018
@jemc
Copy link
Member

jemc commented Oct 3, 2018

This looks good, but I'd like to have a corresponding tutorial PR ready to merge that fixes the terminology there as well.

@mfelsche
Copy link
Contributor Author

mfelsche commented Oct 6, 2018

This PR: ponylang/pony-tutorial#323 incorporates the mod->rem changes. I merged both together as the partial arithmetic PR contains lots of mentions of the modulo operator and thus changed it there.

@SeanTAllen
Copy link
Member

@mfelsche can you add release notes to this PR please?

@SeanTAllen SeanTAllen merged commit aabc3e6 into master Oct 11, 2018
@SeanTAllen SeanTAllen deleted the mod-rem branch October 11, 2018 00:32
ponylang-main added a commit that referenced this pull request Oct 11, 2018
@SeanTAllen SeanTAllen mentioned this pull request Oct 11, 2018
@mfelsche
Copy link
Contributor Author

Release Notes Draft

This change renames the % operation from mod, standing for Modulo, to rem, standing for Remainder. The reason is that % in Pony was actually computing the remainder, not the modulo. The difference is obvious when handling signed integers:

The result of the Modulo operation always has the sign of the divisor, e.g.: -5 mod 2 = 1.
The result of the Remainder operation always has the sign of the dividend, e.g.: -5 rem 2 = -1.

Pony does the latter. We are working on adding a proper Modulo operator later on.

This is a breaking change, if you used one the mod function on Integers or implemented your own mod operator on your data types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants