Xamarin.forms: [Bug] ResourceDictionary.ContainsKey doesnt search MergedDictionaries

[jonkas2211]

Description

The ResourceDictionary.ContainsKey(..) method does not search in MergedDictionaries.
But if you use the indexer (ResourceDictionary[..]) it will search the merged ones.

Steps to Reproduce

1. Add a merged Dictionary to the Application one.
2. Add a KeyValuePair to the Dictionary that is merged.
3. Check if the Key exists.

Expected Behavior

The ContainsKey method should return true.

Actual Behavior

The ContainsKey method return false.

[hartez]

@jonkas2211 Can you provide a small repro project that demonstrates the behavior you are seeing?

[jonkas2211]

Hello @hartez,
I provided you the requested repro project. Take a look at the MainPage .OnAppearing method.

protected async override void OnAppearing()
{
    var dict = new ResourceDictionary();
    dict.Add("MyKey", Color.Red);

    App.Current.Resources.Add(dict);


    if (App.Current.Resources.ContainsKey("MyKey"))
    {
        await DisplayAlert("", "ContainsKey", null);
    }

    if(App.Current.Resources["MyKey"] != null)
    {
        await DisplayAlert("", "Indexer found", "OK");
    }
}

My current workaround for this, is a extension methods. This method looks up all merged dictionaries recursively.

EDIT: Oh I see i made a mistake :)... If the ContainsKey method would be true, the app would crash^^.

Issue11036.zip

[makeman-p81]

This bug related with #10834

ContainsKey and Indexer implementations doesn't support recursion into MergedDictionaries but TryGetValue does it well.

As @StephaneDelcroix said

we know about 1/, it has already been reported (and closed). This is an implementation choice, and we can not change that without it being a breaking change. TryGetValue and {StaticResource} resolutions are recursive, indexer is not.

Possible, it is implementation choice but it is a very unexpected behavior because WPF, UWP and even Silverlight support it as well.

Moreover this issue is very simple for fix. I use customized Xamarin.Forms builds (which contains this fix) for a real app and it works fine.

IMHO Adding of recursion for ContainsKey and Indexer is very soft breaking change because it implies expansion of functional. Yes, we may break hypothetical test cases which checks unreachability of merged resources into merged dictionaries via ContainsKey and Indexer, but into majority of real practical cases full support of recursion is very useful and value.

[programmation]

I realise I'm only one voice but we have so many ways to mitigate the impact of breaking changes that I can no longer support the idea that we should avoid them at all costs like this. IMO the current code is already broken, so we're in a damned-if-you-do/damned-if-you-don't situation. Others may disagree, but I think that bug-free code that behaves as expected is always better than broken-but-compatible code that preserves old mistakes. What motivation do we have for upgrades if obvious bugs like this are not going to be fixed?

[jonkas2211]

Well another way could be a parameter for the ContainsKey method, that indicates, that a recursive search should be performed.

[makeman-p81]

@jonkas2211 please, note that this issue related with indexer too. The fix is very simple for both, should be used TryGetValue as base for indexer and ContainsKey.

[IndexerName("Item")]
public object this[string key]
{
    get => TryGetValue(key, out var value) // recursive
        ? value
        : throw new KeyNotFoundException($"The resource '{key}' is not present in the dictionary.");
    set
    {
        _innerDictionary[key] = value;
        OnValueChanged(key, value);
    }
}

public bool ContainsKey(string key) => TryGetValue(key, out var _); // recursive