search

rsdn / CodeJam

Set of handy reusable .NET components that can simplify your daily work and save your time when you copy and paste your favorite helper methods and classes from one project to another
MIT License
258 stars 35 forks source link

[BreakingChange] Fix AdjustTimeout() method #137

Closed ig-sinicyn closed 3 years ago

ig-sinicyn commented 3 years ago

This PR fixes a bug in timeSpan.AdjustTimeout(TimeSpan maximumValue) overload. The AdjustTimeout() helper is meant to be used for user-specified timeouts to make it safe to pass timeout value into FW methods. As example, Task.Delay(timeout) will throw if the timeout is negative and not equal to -1ms.

As a bonus, there was .AdjustTimeout(maximumValue) overload that ensure that timeout will never be larger than maximumValue arg. This design was counterintuitive as customers did expect behavior similar to .GetValueOrDefault() method.

Current implementation fixes the WTF moment and changes behavior of the overload to match user expectations. It now works as .AdjustTimeout(defaultValue). This is a breaking change but it is much less traumatizing for a new customers.

Old behavior moved into its own AdjustAndLimitTimeout() method.

NN--- commented 3 years ago

Unrelated to this PR , it feels that infiniteIfDefault is not so descriptive. It simply means treat Zero or negative as Infinite.

ig-sinicyn commented 3 years ago

Unrelated to this PR , it feels that infiniteIfDefault is not so descriptive. It simply means treat Zero or negative as Infinite.

@NN--- , I do strongly agree. I was not able to find better argument name so suggestions are welcome!:)

NN--- commented 3 years ago

zeroOrNegativeAsInfinite Better?:)

ig-sinicyn commented 3 years ago

zeroOrNegativeAsInfinite

Well, the name will not match the behavior in case of zeroOrNegativeAsInfinite: false.

The meaning of arg is following

  1. by default: x >=0 - keep as is x < 0 - replace with infinite value (-1ms)
  2. infiniteIfDefault: true: x > 0 - keep as is x <= 0 - replace with infinite value (-1ms)
NN--- commented 3 years ago

Maybe it should have been enum ? :) Let's have it as-is then.

ig-sinicyn commented 3 years ago

Let's have it as-is then.

Ok!:)

andrewvk commented 3 years ago

It's worth fixing readme.txt