whatta / linqbridge

Automatically exported from code.google.com/p/linqbridge
Other
0 stars 0 forks source link

GroupBy fails on null keys #21

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The following test fails with LINQBridge and passes with System.Core:

        [Test]
        public void GroupBy_NullKey()
        {
            var arr = new[] { "notnull", null };

            var groups = arr.GroupBy(s => s);
            foreach (var @group in groups)
            {
                Console.WriteLine(@group.Key ?? "[null]");
            }
        }

Original issue reported on code.google.com by mrdont@mail.ru on 29 Apr 2011 at 9:29

GoogleCodeExporter commented 9 years ago
Proposed solution:

Use as a backing store for Lookup<TKey, TElement> not Dictionary<TKey, ...> but 
Dictionary<Wrapped<TKey>, ...>, where Wrapped<TKey> is some wrapper structure 
that would never be null.

Original comment by mrdont@mail.ru on 29 Apr 2011 at 9:32

GoogleCodeExporter commented 9 years ago
Here is a proof-of-concept implementation of this Wrapped<> structure and it's 
equality comparer.

Original comment by mrdont@mail.ru on 29 Apr 2011 at 10:30

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by azizatif on 30 Apr 2011 at 1:47

GoogleCodeExporter commented 9 years ago

Original comment by azizatif on 30 Apr 2011 at 2:10

GoogleCodeExporter commented 9 years ago
Fixed in changeset c74eb37b988b[1]

[1] 
http://code.google.com/p/linqbridge/source/detail?r=c74eb37b988be35a8a62b7032184
14dc889b5372

Original comment by azizatif on 30 Apr 2011 at 4:20

GoogleCodeExporter commented 9 years ago
The Count property of the Lookup is now broken.
What do you have against wrapping dictionary key in the non-nullable structure? 
This seems to be more "straight solution".

Original comment by mrdont@mail.ru on 30 Apr 2011 at 7:14

GoogleCodeExporter commented 9 years ago
Please, take a look at my solution:
http://code.google.com/r/ilyaindustry-linqbridge-issue21/source/detail?r=00f4036
bafdf260f69e4910006c8c62b931ea18d

PS: Thanks for migrating to mercurial.

Original comment by mrdont@mail.ru on 30 Apr 2011 at 7:40

GoogleCodeExporter commented 9 years ago
> The Count property of the Lookup is now broken.

Good catch.

> What do you have against wrapping dictionary key in 
> the non-nullable structure

Nothing against it. I was trying to solve three problems (order keys for 
GroupBy, null keys and move to Hg) at the same time. I have merged your version 
(thanks) and will be adding back key order for GroupBy; it is required as per 
the documentation:

“The IGrouping(TKey, TElement) objects are yielded in an order based on the 
order of the elements in source that produced the first key of each 
IGrouping(TKey, TElement). Elements in a grouping are yielded in the order they 
appear in source.”

GroupBy internally relies on ToLookup.

Original comment by azizatif on 1 May 2011 at 10:12