SingleRange Begin/End needs to be exposed for GrainServices

[jamescarter-le]

Added ISingleRange interface to be exposed externally. Also exposed helper method NextTimeSpan from StandardExtensions as well as some helper methods in RangeFactory.

[jamescarter-le]

Can be included in a 1.4.X if one of those are going out.

[sergeybykov]

This line would allocate on every call. Can we avoid that?

[jamescarter-le]

We could move it static to the class. Otherwise we could make SafeRandom public so people could keep their own instance. I wanted to expose the NextTimespan method without exposing anything further.

…

On Thu, 16 Mar 2017, 16:47 Sergey Bykov, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/Orleans/Utils/StandardExtensions.cs <#2839 (comment)>: > @@ -39,15 +39,21 @@ public static TimeSpan Min(TimeSpan first, TimeSpan second) return first < second ? first : second; } - public static TimeSpan NextTimeSpan(this SafeRandom random, TimeSpan timeSpan) + public static TimeSpan NextTimeSpan(TimeSpan timeSpan) + { + var random = new SafeRandom(); This line would allocate on every call. Can we avoid that? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2839 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABkUrG6qr20PIGBejRsIKRRDC8HOvkaRks5rmWe8gaJpZM4MaSFO> .

[sergeybykov]

In general, we've been reluctant to expose public extension methods for BCL types, so that we don't appear to be extending functionality of those types: TimeSpan, Task, etc. The concern is that if similar functionality is added at some point to BCL, our extensions will look odd. Of course, we violated that general rule where we deemed it necessary.

In that light, I'm wondering if it's really necessary to expose StandardExtensions and SafeRandom as part of the Orleans API surface.

[jamescarter-le]

Happy to look to modify the way the random time spans can be generated for grain services and either provide a helper method inside GrainService or just leave the user to their own devices. This is for refreshing of the reminder table/user specific implementations.

…

On Thu, 16 Mar 2017, 17:11 Sergey Bykov, ***@***.***> wrote: In general, we've been reluctant to expose public extension methods for BCL types, so that we don't appear to be extending functionality of those types: TimeSpan, Task, etc. The concern is that if similar functionality is added at some point to BCL, our extensions will look odd. Of course, we violated that general rule where we deemed it necessary. In that light, I'm wondering if it's really necessary to expose StandardExtensions and SafeRandom as part of the Orleans API surface. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2839 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABkUrJBSqSB9bndkQEUpz_TjxzfhCG9eks5rmW0lgaJpZM4MaSFO> .

[sergeybykov]

Let's try that approach then. Thanks!

[jamescarter-le]

@sergeybykov I have removed the change to expose the Timer extension method.

[sergeybykov]

Thank you, @jamescarter-le! Sorry about delay.