The documentation is wrong for Timer.Change method #3762
Conversation
The documentation for the following overload of Timer.Change is incorrect: public bool Change (long dueTime, long period); If you use a value for the dueTime parameter that is greater than 4294967294, the method throws an ArgumentOutOfRangeException, not a NotSupportedException as the documentation claims. I'm not sure which exception is thrown if the period parameter is greater than 4294967294. I didn't test that. You might want to check and verify. You should make it clear in the parameter description that values over 4294967294 are not valid. We ran into this problem and it took a little while to debug because the documentation was not clear and plain wrong. Thank you!
mairaw
added
the
✨ 1st-time dotnet-api-docs contributor!
There was a problem hiding this comment.
Thanks for your contribution @brendansensus. I've left some suggestions based on what I'm seeing in the code.
e.g. https://source.dot.net/#System.Private.CoreLib/Timer.cs,810
| @@ -594,7 +594,7 @@ Sub TimerCallback(state As Object) | |||
| <Parameter Name="period" Type="System.Int64" Index="1" FrameworkAlternate="netcore-2.0;netcore-2.1;netcore-2.2;netcore-3.0;netframework-1.1;netframework-2.0;netframework-3.0;netframework-3.5;netframework-4.0;netframework-4.5;netframework-4.5.1;netframework-4.5.2;netframework-4.6;netframework-4.6.1;netframework-4.6.2;netframework-4.7;netframework-4.7.1;netframework-4.7.2;netframework-4.8;netstandard-2.0;xamarinandroid-7.1;xamarinios-10.8;xamarinmac-3.0;netstandard-2.1;netcore-3.1" /> | |||
| </Parameters> | |||
| <Docs> | |||
| <param name="dueTime">The amount of time to delay before the invoking the callback method specified when the <see cref="T:System.Threading.Timer" /> was constructed, in milliseconds. Specify <see cref="F:System.Threading.Timeout.Infinite" /> to prevent the timer from restarting. Specify zero (0) to restart the timer immediately.</param> | |||
| <param name="dueTime">The amount of time to delay before the invoking the callback method specified when the <see cref="T:System.Threading.Timer" /> was constructed, in milliseconds. Specify <see cref="F:System.Threading.Timeout.Infinite" /> to prevent the timer from restarting. Specify zero (0) to restart the timer immediately. This value must be less than or equal to 4294967294.</param> | |||
There was a problem hiding this comment.
If we fix the exception being thrown, do you think it's still useful to mention the max value here?
There was a problem hiding this comment.
I think its still useful to mention the max value in the "dueTime" parameter documentation here. It would probably be good to add the same information on the max value when describing the "period" parameter as well. Not everyone reads all the documentation on the exceptions. I think it makes sense to have information on maximum values when you are describing the parameters. I also looked at your other suggestions above, and they look fine.
There was a problem hiding this comment.
Sounds good. I agree we should do the same for both parameters then. Can you please apply my suggestions above using the Commit suggestion button?
There was a problem hiding this comment.
Yes, I will commit the suggestions now. Wasn't sure if I was supposed to do that as part of the process.
Co-Authored-By: Maira Wenzel <[email protected]>
Co-Authored-By: Maira Wenzel <[email protected]>
now.
The documentation for the following overload of Timer.Change is incorrect:
public bool Change (long dueTime, long period);
If you use a value for the dueTime parameter that is greater than 4294967294, the method throws an ArgumentOutOfRangeException, not a NotSupportedException as the documentation claims. I'm not sure which exception is thrown if the period parameter is greater than 4294967294. I didn't test that. You might want to check and verify. You should make it clear in the parameter description that values over 4294967294 are not valid. We ran into this problem and it took a little while to debug because the documentation was not clear and plain wrong. Thank you!
Summary
The documentation for the Timer.Change method is incorrect. See my description above.
Fixes #Issue_Number (if available)