Supporting C# 7 deconstruction of certain types #14621
Conversation
| IDictionary<TKey, TValue> dictionary = GenericIDictionaryFactory(count); | ||
|
|
||
| Assert.All(dictionary, KeyValuePair_Deconstruct); | ||
| Assert.True(false); |
There was a problem hiding this comment.
I am debugging that xUnit call this test. I will remove this after I see that it fail here. It is just for me.
| { | ||
| IDictionary<TKey, TValue> dictionary = GenericIDictionaryFactory(count); | ||
|
|
||
| Assert.All(dictionary, KeyValuePair_Deconstruct); |
There was a problem hiding this comment.
This would be cleaner if it was just one method. Splitting KeyValuePair_Deconstruct to a helper method is unnecessary unless we're overriding it somewhere.
There was a problem hiding this comment.
It can be used to test deconstruction in other methods. For example I can create test for empty pair deconstruction and use this method in new test. I did not create such testing because it seems useless, but it is good to split one method to logical parts.
There was a problem hiding this comment.
it is good to split one method to logical parts.
Generally yeah, but the calling method is only two lines. We have to be careful with the helper method style because it moves the test logic out of the test itself and makes the code overall more difficult to follow. So unless we have a good reason for it (e.g. other tests that use the same logic like KeyValuePair_ToString), we should keep the code consolidated.
| foreach (DictionaryEntry entry in dictionary) | ||
| { | ||
| DictionaryEntry_Deconstruct(entry); | ||
| } |
There was a problem hiding this comment.
nit: Assert.All.
Also, same comment on above about helper method.
There was a problem hiding this comment.
Assert.All was before here. But it is only for generic and can not be compiled here.
|
@AlexRadch do you plan to finish the work? |
|
The PR seems to be abandoned. Closing for now. |
C# 7 added support for deconstruction of user-defined types via a Deconstruct(out ...) method. It would make sense to be able to use this feature with tuple-like types such as KeyValuePair and DictionaryEntry.
Close https://github.com/dotnet/corefx/issues/13746