language-ext icon indicating copy to clipboard operation
language-ext copied to clipboard

Fix Distinct with custom comparer (hash bug)

Open StefanBertels opened this issue 2 years ago • 0 comments

There is a bug in .Distinct() when a custom comparer is used. PR includes test and fix.

Bug is that default GetHashCode is used -- this is not valid with a custom (key) comparer.

First both commits only fix one occurence of the bug (there are more). Some things come to my mind (before adding more tests/fixes):

  1. IMHO there should be a Distinct<EQ, T, K>(...) variant which I would prefer for my use case (avoiding runtime comparer).
  2. Maybe avoid 'func parameter' style Func<string,string,bool> for helper functions like this (comparer). This would avoid hiding a gethashcode "problem" (either needs to be supplied or otherwise risks performance penalty). Should be enough to allow a variant with IEqualityComparer<T> parameter and make something like EqCompare<T> public to allow on-the-fly creation of Comparers using equals+gethashcode.
  3. Do you mind merging https://github.com/louthy/language-ext/pull/841 (which is related here / giving wrapper EQ => IEqualityComparer)? This would be a (semi) replacement for (1.)

PS: In case you like to stay with 'comparer func parameter': Maybe code (and bugfix) should be improved by moving IfNone and Match outside to avoid calling this within each call of of the comparer. Maybe comparer the should be non-optional (adding another signature without comparer) to clean up code.

StefanBertels avatar Jun 30 '22 09:06 StefanBertels