PersistentHashMap.containsKey is broken for null values about fsharpx.collections HOT 5 OPEN

I think this is an issue with the HashMap architecture, and probably has a similar issue with keys.
The only fix I can think of would be altering the underlying Array to bool * obj pairs to indicate presence. I don't know if that would impact performance enought to make it an undesirable change.

The other alternative is to find an elegant way to prevent option types.

from fsharpx.collections.

I've added pending Expecto tests in https://github.com/fsprojects/FSharpx.Collections/blob/master/tests/FSharpx.Collections.Tests/PersistentHashMapTest.fs and https://github.com/fsprojects/FSharpx.Collections/blob/master/tests/FSharpx.Collections.Tests/TransientHashMapTest.fs

from fsharpx.collections.

Thanks.

bool * obj sounds sensible. Maybe obj option could also work, because Some null cannot be represented by null.

Or if the (potential) performance is a problem, we could just exception guard the add function.

On your suggestion of preventing option types, we could add a and 'S : struct constraint. Unfortunately, that would be overly restrictive, given that unions do not default to being structs.

It is a shame that F# has a null constraint, because what we really need is a not null constraint. Edit: actually, that would not help either. null is not a "proper value" of an option type, despite null being an internal representation of None.

from fsharpx.collections.

This is terrible. It is quite disappointing when the Option type fails at the only thing it's meant to do. Hope it gets fixed.
In the meantime here is a hack for anyone who still wants to use Options.
(It should only override functionality for case when you use Options and no other case)

type PersistentHashMap() =
    static member containsKey key hm = try hm |> PersistentHashMap.find key |> fun x -> Option.isSome x || Option.isNone x with _ -> false

from fsharpx.collections.

It looks to me like the problem here is with INode.find. Note that Clojure has two overloads for this method; the one used for containsKey takes a distinct NOT_FOUND object which is used to detect when the key is not found. Anyway, it looks like INode.find (in current FSharpx.Collections) is only ever used by ContainsKey, so why not change the result type of INode.find to bool and make the appropriate changes to the node implementations? Since INode is internal to the library, I believe that shouldn't be a breaking change.

from fsharpx.collections.