NMFCode / NMF

This repository contains the entire code for the .NET Modeling Framework
BSD 3-Clause "New" or "Revised" License
36 stars 15 forks source link

NMF Expressions - Feature idea - implement `ToLookup` #39

Closed mcintyre321 closed 6 years ago

mcintyre321 commented 6 years ago

I think it would be great to have a ToLookup implementation. I have been using NMF to create in memory projections from an Event Store, and having a ToLookup implementation would enable me to find the items by key, like a DB index.

I think this is a very neat project by the way. I was using OLinq, but switched my code over once I discovered it. I think the uptake may be hampered a little bit by seeming like a component of the larger NMF project - I don't know if you would consider splitting and rebranding it with a catchy name, e.g. IncrementalLINQ or something like that?

georghinkel commented 6 years ago

Sorry for the missing response, I somehow did not see the issue (the email that I got from Github somehow was lost somehow).

ToLookup will be interesting and I guess not even so hard to implement, I will have a look at it.

Regarding rebranding, the original name of this was actually ObservableLinq, but it turned out that this caused a lot of confusion with Rx. Therefore, I renamed it and made it part of my other, larger, project.

georghinkel commented 6 years ago

Please check whether 6e57f3c satisfies your needs

mcintyre321 commented 6 years ago

Hi thanks for looking at this George.

I just downloaded the latest package 2.0.128 and tried this in LinqPad:

void Main()
{
    var oc = new ObservableCollection<Customer>();

    var customersByLocationLookup = oc.WithUpdates().ToLookup(o => o.Location);
    var r = new Random(1);

    for(var i=0;i<10000;i++)
    {
        var c = new Customer(){ Name = "Customer " + i, Location = "Location " + r.Next(50) };
        oc.Add(c);
    }

    oc.Where(o => o.Location == "Location 30").Count().Dump(); //202

    customersByLocationLookup["Location 30"].Count().Dump(); //was 0 but expected 202
}

public class Customer 
{
    public string Name { get; set; }
    public string Location { get; set; }
}

Should that have worked? (i.e. returned 202 results from the customersByLocationLookup["Location 30"] call

georghinkel commented 6 years ago

Actually no, it should not. I had only implemented ToLookup for IEnumerableExpression, not for INotifyEnumerable. The difference between the two is that the latter is fixed to incremental execution whereas the former lets the programmer choose.

I am uploading a new version that also added your example as a unit test. It does implement ToLookup now also for INotifyEnumerable.

mcintyre321 commented 6 years ago

I've tried with 2.1.129 but it still doesn't seem to return 202 results - is that the version you are referring to?

georghinkel commented 6 years ago

Hm, I actually included your example as a unit test that actually passes: https://github.com/NMFCode/NMF/blob/master/Expressions/Tests/Expressions.Utilities.Tests/LookupTests.cs

One thing, did you add the using statement? Could you check in the IDE that it is really the INotifyEnumerable version of ToLookup that is actually been called and not the IEnumerable version?

friuns2 commented 6 years ago

I was looking whole week for this but found only dead projects like clinq olinq and many others. Good @mcintyre321 pointed me to this project, very hard to find :)

georghinkel commented 6 years ago

Closing this old issue because the requested feature is implemented now

mcintyre321 commented 6 years ago

Sorry @georghinkel , I left you hanging. I never managed to get the snippet to work in Linqpad, after installing NMF-Expressions and NMF-Expressions-Utilities 2.0.129 I couldn't find the NMF.Expressions.Linq.LookupExtensions class in my app (and even with using NMF.Expressions.Linq). Are those the required packages ?

georghinkel commented 6 years ago

Yes, they are the required packages and version 2.0.129 is the correct one. However, I cannot reproduce the issue, if I download the package to a fresh console app, I can very well access the class.

mcintyre321 commented 6 years ago

So I've got to the bottom of why I can't see the class. If you install on .NET Framework, then the NMF.Expressions.Linq namespace isn't available, but if you install on Core it is.

You can check this by going to https://www.fuget.org/packages/NMF-Expressions-Utilities/2.0.129/lib/net45/NMF.Expressions.Utilities.dll/NMF.Expressions/LookupExtensions and clicking between netstandard2.0 and net45, the namespace will disappear when on net45.

georghinkel commented 6 years ago

Ah, sorry, I forgot to add a reference to this implementation for the .NET 4.5 projects. At the time I created support for .NET Standard, I was not aware that a single project could target multiple platforms at the same time. Hence, I created the backport to .NET 4.5 by projects that simply reference the original code. I simply forgot to add those references.