- 
                Notifications
    You must be signed in to change notification settings 
- Fork 6.1k
Rename "modulus" to "remainder" #4860
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
Conversation
        
          
                docs/csharp/language-reference/operators/multiplication-operator.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making these updates. We really appreciate it. 👏
I've reviewed the updates. I agree with the comments made so far, and added a few suggestions.
Just @ - mention us when you've updated it, and I'll review again and merge it.
        
          
                docs/csharp/language-reference/operators/remainder-assignment-operator.md
              
                Outdated
          
            Show resolved
            Hide resolved
        
      | @BillWagner @cartermp should those two F# pages be updated with modulus to remainder changes as well? | 
| @aaronfranke as for the Decimal doc changes, the description of the Modulus operator should be updated. That's pity we cannot change the name of the operator, especially if you know that this operator does the same as the Remainder method :) If you want to update that part of documentation, make a PR with updates to the Decimal.xml file in the API reference repo | 
| @BillWagner @pkulikov I've made several changes, let me know if there are any further concerns. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it would be preferable to use the same definition that is used in both the C# spec and the IL spec - a % b = a - (a / b) * b.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with either, as long as it's consistent. (I used the formula I did because it's used above as well.)
BTW, where is that formula in the C# spec? Looking here in the draft C# spec, I find:
The result of x % y is the value produced by x - (x / y) * y. If y is zero, a System.DivideByZeroException is thrown.
for integer remainder and:
z is the result of x % y and is computed as x - n * y, where n is the largest possible integer that is less than or equal to x / y.
for floating point remainder.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, where is that formula in the C# spec?
I'm not sure I understand the question. It seems that you already found it.
| @pkulikov I'd approve of making similar changes to those F# docs. Absolutely feel free to include in this PR 😄 | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've had just one remark on wording. @aaronfranke could you please also update F# articles mentioned in the comment above. I guess Visual Basic docs also might contain some modulus inclusions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
floating types - I'm not sure if that's correct; let's just simplify:
...use float or double type. To do that, append .0 to a number literal as the example below shows.
(Append always adds to the end by definition).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The suggestion to append .0 is slightly misleading, that will produce a double literal, you need .0f to produce a float literal. It may be better to simply state "use floating point types"  instead of going into details about how to produce a floating point literal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say that a documentation page about an operator is not the place to explain how a floating point literal is formed. Simply hinting the user to use floating point should be enough.
But then I do not know the intended target audience of this documentation. Something tells me that it may have been intended for beginners, there are too many cases where some things are over simplified to the point that they're wrong while other things are, not sure what word to use for it, accentuated, redundant etc.
I supposed that for beginners it may be worth reminding them how floating literals are formed, many do not realize that there is a difference between 2, 2.0f and 2.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in this case we can omit float type mention and only refer to double as it's about having a quotient non-integer. OR: remove remark on appending: if both operands are not number literals, that remark is not correct and casting should be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good discussion.  I'll suggest not using .0 at all, as that works on literals, but does not on variables. For the second sentence, I'd say:
There are many ways to convert between built in numeric types
Note that I used the internal link in my comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'll make the first part say ", use the float, double, or decimal types." I was intentionally vague with "floating types" because there is more than just float and double, as decimal exists.
| So it looks like I should edit the F# docs too, and maybe VB ones? Anything else? As I'm not familiar with these languages I'd like to confirm they have remainder behavior. | 
| @aaronfranke Both indeed have remainder behavior. | 
| @aaronfranke @cartermp Yes, the VB and F# docs need updating as well. You can add them to this PR, but if you don't feel comfortable in those languages, or don't have time, we'll make an issue and address those changes ourselves. Thanks for all your work on these docs. | 
| I have decided to NOT edit the Visual Basic docs as I'm not familiar enough with the language and the situation seems to be a bit more complicated (the operator appears to be the text "Mod" itself). I was, however, delighted by how easy it was to fix up the F# docs (@cartermp). I also made a few more tweaks as @mikedn @BillWagner @pkulikov suggested. I believe this PR is good to merge. (after #4789 ?) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aaronfranke that looks good to me. Thank you very much for all the effort to spot the issue, discuss it, and fix the docs at the end!
| Thanks @aaronfranke I created Issue #4884 for VB. | 
| @aaronfranke Thank you for sparking this discussion, participating and investigating the changes needed, and making the edits for C# and F#. We really appreciate it. 👏 Thanks for the kind words about how easy it is to participate. We're working to make it as easy as possible, and my team is glad we're on the right track. Thanks again. I'll  | 
 now.  You should see the changes on the live site in the next day or two, on our regular publishing cycle.
 now.  You should see the changes on the live site in the next day or two, on our regular publishing cycle.
I've renamed "modulus" to "remainder", added clarification for these terms, added clarification for division in C#,
added a tip for how to use, and made a few other minor tweaks.&Part of fixing #4827 but I'm not sure what to do about the Decimal docs: #4827 (comment)