ListDictionary TKey and TValue same type

[DancePanda42]

Package info

• Platform: Windows
• Prism.WPF version: 6.3.0

Repro steps

with the same "key" and "value" type you cannot use the Remove( TKey ) or Remove( TValue ) method

[DancePanda42]

this is my fix

[brianlagunas]

Could you provide more detail? I have no idea what you are talking about or what you are trying to do.

[dvorn]

This is about the Prism.Common.Listdictionary class in Prism.Wpf. When TKey and TValue is the same type, the methods Remove(TKey key) and Remove(TValue value) in ListDictionary<TKey, TValue> become ambiguous.

[brianlagunas]

This is not a collection meant to be used generically. These methods also work as intended for its uses within Prism.

[brianlagunas]

Also, I think you misunderstand what those methods are doing. When you use the (key, value) method, you are removing values from the list with the given key, not actually removing the list.

This is described in the method comments:

 /// <summary>
        /// Removes a value from the list with the given key.
        /// </summary>
        /// <param name="key">The key of the list where the value exists.</param>
        /// <param name="value">The value to remove.</param>

What you did was create an Extension method that actually removed the list from the collection, which is not the intended function of the method you are using.

[dvorn]

This is not a collection meant to be used generically.

Then we should make it internal.

[brianlagunas]

Once you make something public for nearly 10 years, you can't just make it internal. Also, the collection is working exactly as it should. The OP did not understand what those methods actually did.

[dvorn]

These methods also work as intended for its uses within Prism.

Also, the collection is working exactly as it should.

The API of Listdictionary class is poorly designed. Listdictionary<string, string> works in Prism.Modularity.ModuleDependencySolver only because it never attempts to remove neither key nor value.

[brianlagunas]

Possibly, but this really is a non-issue.

[dvorn]

Sure, not a big deal. But the library provides public general-purpose collection class, it does not behave well, and the users complain.

This can be fixed without breaking backward compatibility by adding RemoveKey(TKey) and RemoveValue(TValue) methods which will do exactly what Remove(TKey) and Remove(TValue) do.

[brianlagunas]

The methods in the current API work as expected. The remove Key/Value will first get the collection based on key, the loop through that collection and remove the value from that collection.

I am confused on what needs to change.

[dvorn]

After the line https://github.com/PrismLibrary/Prism/blob/master/Source/Wpf/Prism.Wpf/Modularity/ModuleDependencySolver.cs#L59 add the line

dependencyMatrix.Remove(module);

and try compile.

[brianlagunas]

So your main complaint is that the method Remove(value) doesn't accurately represent what the method does. Instead RemoveValueFromLists(value) would make more sense?

[dvorn]

Did you tried to compile?

[brianlagunas]

No, I am working on other, more important, things :)

[dvorn]

The problem is not the meaningful name. It does not compile. Does not work at all.

[brianlagunas]

Ahh, now I see what you mean. It is because of the method name :)

Should be named RemoveValue instead. Actually looks like the easy fix is for both of the methods to be called RemoveValue.

[dvorn]

No, method names should be different. The problem is that there exist two methods with the same name (Remove) which differ only in argument type (TKey/TValue). This becomes ambiguous when TKey and TValue are the same type.

[brianlagunas]

Yeah that's what I meant.

RemoveValue(TKey, TValue) and RemoveValue(TValue)

That would fix the issue.

[dvorn]

But this will be a breaking change. Easy to fix though.

[brianlagunas]

The good news is I don't think many people are using this anyways. In 10 years this is the first I have heard of anyone using it. It will be noted as a break, and it will result in a build error so anyone using it will know where the break is. If it was a behavior break, those are nearly impossible to find and I try to avoid those at all costs.

[brianlagunas]

Fixed

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.