elucidsoft / dotnet-stellar-sdk

Stellar API SDK for .NET 6.x
Apache License 2.0
117 stars 54 forks source link

Being able to create a map whose key is `KeyPair` would be really nice #309

Closed hidenori-shinohara closed 3 years ago

hidenori-shinohara commented 3 years ago

Is your feature request related to a problem? Please describe. I'd like to create a map of KeyPair objects. When I try to do so, I get the following message:

/home/hidenori/some/directory/filename.fs(60,17): error FS0001: The type 'KeyPair' does not support the 'comparison' constraint. For example, it does not support the 'System.IComparable' interface

This feature would let me create a container of information of nodes indexed by KeyPair. Being able to create something like configInfoMap: Map<KeyPair, Config> would make it easy for me organize my code.

Describe the solution you'd like If KeyPair supports the comparison constraint (e.g., by comparing the corresponding byte arrays), it'll let me create a map whose keys are KeyPair.

Describe alternatives you've considered One workaround I've found is to use byte[] that I get by calling the .PublicKey method. However, it makes code much less clear (e.g., It's unclear what the key in configInfoMap: Map<byte[], Config> is).

Additional context Nothing!

fracek commented 3 years ago

Good question. I think we can implement IComparable<T> on KeyPair, there is one case where I'm not sure what is the correct thing to do. Let's say we have a KeyPair that contains both the public and secret key, and another that contains only the public key (same public key as the other key pair). I think we can agree that in this case the keys are different, but which one is "greater than" the other? If, on the other hand, implement only IEquatable<T> then we don't have this problem but you cannot use it as key on a Map.

hidenori-shinohara commented 3 years ago

Thanks for the reply! That is a good point, I never thought of that case.

Yes, I agree that the keys aren't the same. I'm not 100% sure which one would make more sense (public key > pair vs public key < pair).

I wonder if the pair should be greater because one can think of it as comparing concat(publicKey, privateKey) with publicKey and apply the same principle that gets applied when comparing two strings where one contains the other (e.g., code < coder, we < weather) But I have no strong feeling about it. So far, I've never encountered a situation where I wanted to compare two KeyPair objects and my primary motivation is to use it inside a Map and I believe either ordering would work just as fine for that purpose 👍

fracek commented 3 years ago

I started working on this and I think implementing IComparable<T> is a mistake, I think we should stick to IEquatable<T>. In F# you can use FSharpx.Collections to have an immutable hashmap with non-comparable keys.

hidenori-shinohara commented 3 years ago

Thank you for taking care of this so quickly!

elucidsoft commented 3 years ago

Bump

fracek commented 3 years ago

Need to make a release and publish it to nuget.

Kirbyrawr commented 3 years ago

We can close this as we have now released it.