Bug in random string generation for targets > NET Framework 2.0. Even with forceUnique set to false, get unique strings

[jshergal]

Expected behavior is that if I set forceUnique to false, then I will end up with a resultant collection containing count strings.

Currently, when the target framework is > .NET Framework 2.0, strings are generated and added into a HashSet. This automatically creates a collection of unique items, whether the caller wanted unique or not.

It is easy to test/reproduce with this line of code:
List<string> randomStrings = RandomString.GetStrings(Types.NUMBERS, count: 2000, maxLength: 3, randomLength: false, forceUnique: false);

See:

RandomString4Net/RandomString4Net/RandomString.cs

Lines 188 to 205 in f01e288

	HashSet<string> randomStrings = new HashSet<string>();
	#endif
	int inputStringLength = inputString.Length;
	int outputStringLength;
	for (int i = 0; i < count; i++)
	{
	outputStringLength = randomLength ? randomInstance.Next(1, maxLength) : maxLength;
	StringBuilder currentRandomString = new StringBuilder();
	for (int j = 0; j < outputStringLength; j++)
	currentRandomString.Append(inputString[randomInstance.Next(inputStringLength)]);
	if (forceUnique && randomStrings.Contains(currentRandomString.ToString()))
	continue;
	randomStrings.Add(currentRandomString.ToString());

Proposed solutions:
Instead of using a HashSet, just stick with List:
If the target is .NET Framework 2.0, then use a combination of BinarySearch and Insert to maintain the list in sorted order and ensure only unique items are inserted
For other framework targets, use Enumerable.Distinct() when returning the collection if forceUnique is true.

A micro benchmark would be helpful to judge the performance of the latter option. It might be better to have both the List and the HashSet, checking the HashSet before inserting into the List, but of course there is a memory tradeoff there.

[jshergal]

I ran a quick micro benchmark on .NET 6.0 and here are the results:

Method	Mean	Error	StdDev	Gen0	Gen1	Gen2	Allocated
BenchmarkHashSetThenList	283.4 μs	4.45 μs	3.94 μs	88.8672	38.0859	31.7383	275.9 KB
BenchmarkListDistinct	497.5 μs	5.71 μs	4.77 μs	107.4219	54.6875	33.2031	325.39 KB
BenchmarkListSorted	8,297.9 μs	60.90 μs	47.55 μs	31.2500	15.6250	-	78.21 KB

So it looks like the best option would be using a HashSet for uniqueness and just appending to the final list, skipping that step of course if uniqueness is not required. It does increase the memory profile (as one would expect), but it is the most performant option.

[ljnath]

Good catch ! Feel free to raise PR for the same.

[jshergal]

Hi @ljnath,
I have fixed it in my own fork here, you are welcome to implement it:

jshergal/StuceSoftware.RandomStringGenerator@2051e22

[jshergal]

As a side note, related to my other comments regarding continuing to support frameworks that are out of support. If you drop the .NET Framework 2.0 and 3.0 support, and just maintain support back to 3.5 SP1, then you can easily merge this change into your repo. In order to use HashSet you have to be on at least .NET Framework 3.5.