Improved RandomHelper

[MarkCiliaVincenti]

Internally uses Random.Shared when available (.NET 6.0), otherwise uses a thread-safe random.

Furthermore, list shuffling uses the Fisher-Yates algorithm for better performance.

[MarkCiliaVincenti]

@maliming I'm OK with rewriting this in such a way to avoid the dependency, basically copying and pasting the code in and adding a comment at the top for credit, and we can leave them public as well. Would that work for you?

[maliming]

@maliming I'm OK with rewriting this in such a way to avoid the dependency, basically copying and pasting the code in and adding a comment at the top for credit, and we can leave them public as well. Would that work for you?

That would be great. 👍

[MarkCiliaVincenti]

@maliming I'm OK with rewriting this in such a way to avoid the dependency, basically copying and pasting the code in and adding a comment at the top for credit, and we can leave them public as well. Would that work for you?

That would be great. 👍

I tried to do this but unfortunately this should be multi-targeting a framework version that is not being targeted in Volo.Abp.Core, namely .NET 6.0 where Random.Shared was introduced. It will also use the internal Shuffle of .NET 8.0 but that's not an issue as .NET 8.0 is currently being targeted, but will it remain being targeted?

It would be best to have these micro optimizations handled externally in a dedicated and controlled assembly.

Therefore my recommendation is to optimally take the micro-dependencies.

[maliming]

hi @MarkCiliaVincenti

These changes will be easier to maintain and understand. What do you think?

https://github.com/abpframework/abp/pull/21089/files

https://stackoverflow.com/questions/1287567/is-using-random-and-orderby-a-good-shuffle-algorithm/1287572#1287572

---

Also applies to https://github.com/abpframework/abp/pull/20530

[MarkCiliaVincenti]

hi @MarkCiliaVincenti

These changes will be easier to maintain and understand. What do you think?

https://github.com/abpframework/abp/pull/21089/files

https://stackoverflow.com/questions/1287567/is-using-random-and-orderby-a-good-shuffle-algorithm/1287572#1287572

Also applies to #20530

There are some changes that should be done here.

1. I don't quite like the yield return and returning an IEnumerable because there's a risk that someone keeps it as an ienumerable and it's reprocessed every time it's accessed... and in such case the shuffled version would be different on top. Very dangerous.

2. Instead of using a static Random instance and locking on it, you can optimize it with a ThreadLocal. Take a look at https://github.com/MarkCiliaVincenti/ThreadSafeRandomizer/blob/master/ThreadSafeRandomizer/ThreadSafeRandom.cs

3. GenerateRandomizedList now has a breaking change. With the current code, the input (items) is not shuffled, but with the version in #21089 items will get shuffled.

4. NET 8.0 already includes a shuffle and we should use that.

If you want I can modify this PR and import the two libraries here in separate files. They won't be fully optimized due to the lack of extra targeting, but it will work.

As for #20530, please discuss there. The changes absolutely need to be in a different assembly there.

[maliming]

Thanks. I will check it later.

[hikalkan]

@MarkCiliaVincenti thanks for PR. However, we won't merge that. Because, we don't want to introduce 2 package dependencies for such small functionality. If you directly implement a similar code in the ABP Framework, it is appreciated.