Editing BiDictionaries a little unsafe

[charlesroddie]

The following code occasionally causes a crash on Android.

do  let colDict = CSharpMath.Structures.Color.PredefinedColors
colDict.Remove("green",CSharpMath.Structures.Color(0uy,128uy,0uy)) |> ignore
colDict.Add("green",CSharpMath.Structures.Color(130uy,212uy,20uy))

Error report from @kevcrooks

The exception is System.ArgumentException: Duplicate second Parameter name: second
My thought is perhaps this line of code is being run twice, if a user has closed and re-opened the app or something.

This is fixed by changing to:

do  let colDict = CSharpMath.Structures.Color.PredefinedColors
colDict.Remove("green",CSharpMath.Structures.Color(colDict.["green"]) |> ignore
colDict.Add("green",CSharpMath.Structures.Color(130uy,212uy,20uy))

But it would be better for .Remove in a BiDictionary<'A,'B> only to take one argument which would increase safety, and also for .Add not to throw exceptions.

[FoggyFinder]

Current implementation of the Remove method is really unsafe. A problem here is that second parameter may not be exists in the corresponding dictionary. So the second call of the Add method will throw an exception as mention in the issue.

So it would be better to change it to something like this:

    public bool Remove(TFirst first, TSecond second) {
      if (firstToSecond.ContainsKey(first) && secondToFirst.ContainsKey(second)) {
        return firstToSecond.Remove(first) && secondToFirst.Remove(second);
      }
      return false;
    }

Though methods like

• Remove(TFirst first)
• Remove(TSecond second)
• AddOrReplace(KeyValuePair<TFirst, TSecond> pair)
• AddOrReplace(KeyValuePair<TSecond, TFirst> pair)

would be really nice to have.

[kevcrooks]

For our implementation, we need to AddOrUpdate the value of green in the dictionary, so would be happy with a public-facing method that can do this based on a key and a new value to insert.

The main outcome we need is for the resulting dictionary to be guaranteed to have the mapping ("green",CSharpMath.Structures.Color(130uy,212uy,20uy) after the call (and for this not to throw).

[FoggyFinder]

What is the reason to have BiDictionary for PredefinedColors? I haven't found any code related to getting a name from the color, so maybe it will be better to change it to usual Dictionary?

[Happypig375]

The current Color atom should be changed to store a CSharpMath.Structures.Color, which will use this functionality.