Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GetCitiesFromEmbeddedResource() enumerates forever #2828

Closed
JunSota opened this issue Dec 26, 2024 · 2 comments
Closed

GetCitiesFromEmbeddedResource() enumerates forever #2828

JunSota opened this issue Dec 26, 2024 · 2 comments
Labels

Comments

@JunSota
Copy link

JunSota commented Dec 26, 2024

Mapsui Version
5.0.0-beta.6

Mapsui Platform
WindowsForms

Device
Windows 11

Describe the bug
I have a question about using Mapsui.Samples.WindowsForms.

When running Geometries -> Points in the app, it continuously enumerates the contents of

Mapsui.Samples.Common.GeoData.Json.congo.json

forever at the part

    return cities.Select(c =>
    {
        :
        :
        return feature; 
    }

in method

private static IEnumerable GetCitiesFromEmbeddedResource()

in file

Mapsui.Samples.Common\Maps\Geometries\PointsSample.cs

Is this the intended behavior?

To Reproduce

  1. I added console logging before "return feature;"
    return cities.Select(c =>
    {
        var feature = new PointFeature(SphericalMercator.FromLonLat(c.Lng, c.Lat).ToMPoint());
        feature["name"] = c.Name;
        feature["country"] = c.Country;
        Console.WriteLine("Name={0}", c.Name);  // FOR DEBUG LOGGING
        return feature;
    });

I also added
AllocConsole();
at the main program.

  1. Start application, select "Geometries" and click "Points" radio button.

  2. Log shows enumerates forever.

@pauldendulk
Copy link
Member

Interesting, I think this is what resharper calls 'possible multiple enumerations' https://www.jetbrains.com/help/resharper/PossibleMultipleEnumeration.html#false-positives

The solution would be to add a .List() like this:

    return cities.Select(c =>
    {
        var feature = new PointFeature(SphericalMercator.FromLonLat(c.Lng, c.Lat).ToMPoint());
        feature["name"] = c.Name;
        feature["country"] = c.Country;
        Console.WriteLine("Name={0}", c.Name);  // FOR DEBUG LOGGING
        return feature;
    }).ToList();

I see we have three versions of the method, and perhaps other methods with a different name but the same problem.

The annoying thing of the 'possible multiple enumerations' warning is that you get it in every method that receives a IEnumerable<> even if the providing method already turned it into a List. The guideline I use is that the source is responsible to do a .ToList(). So, in this case the GetCitiesFromEmbeddedResource() method has a bug.

The advantage of the IEnumerable over List is that you can not modify it. An alternative can sometimes be to use the new .NET Span or Memory types.

@JunSota
Copy link
Author

JunSota commented Jan 7, 2025

By adding .ToList(), the issue was perfectly resolved.
Since I'm not very familiar with the behavior of .NET, I couldn't figure out the cause until I received your reply.
It was a great learning experience.
Thank you very much.

@JunSota JunSota closed this as completed Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants