Runtime: Add bool TryGetValue(TKey, out TItem) method to a KeyedCollection<,> class

[ViIvanov]

Suggested TryGetValue method allows to check a collection for an "item" with specific "key" and get item if it exists in the collection.
Similar that occur in the Dictionary<,>.

public bool TryGetValue(TKey key, out TItem item)
{
    if (key == null)
    {
        throw new ArgumentNullException("key");
    }

    if (_dict != null)
    {
        return _dict.TryGetValue(key, out item);
    }

    foreach (TItem itemInItems in Items)
    {
        TKey keyInItems = GetKeyForItem(itemInItems);
        if (keyInItems != null && _comparer.Equals(key, keyInItems))
        {
            item = itemInItems;
            return true;
        }
    }

    item = default(TItem);
    return false;
}

If some existing derived type "MyKeyedCollection<,>" already contains the same method, only warning occurred in compilation:

warning CS0108: 'MyKeyedCollection. TryGetValue(TKey, out TItem)' hides inherited member 'KeyedCollection. TryGetValue(TKey, out TItem)'. Use the new keyword if hiding was intended.

[Priya91]

Sounds reasonable to me.

[weshaggard]

We reviewed and like adding an API for the scenario but for consistency with other similar APIs in the framework we believe this should be TryGetValue instead of TryGetItem, please update the proposal accordingly.

[ViIvanov]

@weshaggard Thank you! Should I update my start topic with approved name of the method?

[Priya91]

Thank you! Should I update my start topic with approved name of the method?

Yes, please update the speclet, the api is tagged with api-approved, so you can put up a PR if you're willing to contribute.

[ViIvanov]

@Priya91, @weshaggard Should I send my PR to the master branch or to the feature branch, because I "adding API to a type that exists in the full .NET Framework"? (https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/branching-guide.md)

[Priya91]

The PR should be towards master branch, we'll be merging future to master soon, and then we'll mark this issue with netfx-port-consider label, to add this API back to Desktop.

[karelz]

@ViIvanov do you plan to work on it? Would an example help?

[ViIvanov]

@karelz I'm sorry for big delay. I trying to understand how the unit tests works and how to write new one. Will try to send PR today.

[karelz]

@ViIvanov no worries, there's no rush. I just wanted to know if I should assign it to you ...

[ViIvanov]

@karelz, Thank you.

I need some help. I have added a new method to KeyedCollection<,> class (at srcSystem.ObjectModelsrcSystemCollectionsObjectModelKeyedCollection.cs and .srcSystem.ObjectModelrefSystem.ObjectModel.cs).

Cd ".\src\System.ObjectModel\src\"
MsBuild

and

Cd ".\src\System.ObjectModel\ref\"
MsBuild

builds succesfully. But when I run

Cd ".\src\System.ObjectModel\tests\"
MsBuild

I get errors:

ValidateApiCompatForSrc:
  ".\Tools/dotnetcli/dotnet" ".\Tools/ApiCompat.exe" @".\bin/obj/AnyOS.AnyCPU.Debug/System.ObjectModel/net463/apicompat.rsp"
.\Tools\ApiCompat.targets(68,5): error : MembersMustExist : Member 'System.Collections.ObjectModel.KeyedCollection<TKey, TItem>.TryGetValue(TKey, TItem)'
 does not exist in the implementation but it does exist in the contract. [.\src\System.ObjectModel\src\System.ObjectModel.csproj]

Same error I see when I try to build .srcSystem.ObjectModelsrcSystem.ObjectModel.csproj in configuration net463_Debug
I have written test for a new method but I cannot run it.

What can I try to do for fix this issues and run tests?

[karelz]

net463 configuration should not be affected - you probably change the wrong contract. Can you publish your changes?
@Priya91 @ianhays can you please help here?

[ianhays]

@ViIvanov when you build the src and ref, make sure to build the *builds file e.g.

Cd ".\src\System.ObjectModel\ref\"
MsBuild *.builds

The reason you're seeing that error in tests but not src/ref is because the tests build the src/ref for more configurations than just the default.

As Karel mentioned, if you post your branch I can guide you a bit better.

[Priya91]

Seems like you've added the api for all configurations in ref.. Make sure it's included only for netcoreapp1.1 profile, by using a build constant like here

[ViIvanov]

@karelz, @ianhays, @Priya91 Thanks for you suggestions, but nothing of them helps me. I have posted my PR as is and ready to fix issues.