CODEWITHSUNDEEP
c#linq
Soatl
Soatl

Reputation: 10592

List becomes empty when ordered

I have the following method:

    public List<Vehicle> Vehicles
    {
        get { return _vehicles; }
    }

Which will return data. If I do the following;

    public List<Vehicle> Vehicles
    {
        get { return _vehicles.OrderBy(v => v.Year).ToList(); }
    }

It becomes empty. If I call the orderby outside this method myList = vehicleList.Vehicles.OrderBy(x => x.Year) the list will contain all needed data.

I am confused as to why.

Upvotes: 0

Views: 158

Answers (2)

MarcinJuraszek
MarcinJuraszek

Reputation: 125650

I think you're missing a fact, that ToList() creates new List<T> instead of returning the underlying one.

So when you call:

var list = vehicleList.Vehicles;
list.Add(new Vehicle());
list.Add(new Vehicle());
Console.WriteLine(vehicleList.Vehicles.Count);

It will print 2 for your first Vehicles property declaration and 0 for the second one.

That's because with your second property declaration you're adding items to a List that was just created using ToList() call, not to that one from _vehicles backing field.

To make it work, try following:

public List<Vehicle> Vehicles
{
    get { return (_vehicles = _vehicles.OrderBy(v => v.Year).ToList()); }
}

Or use List<T>.Sort method instead of OrderBy LINQ extension method:

public List<Vehicle> Vehicles
{
    get
    {
        _vehicles.Sort((v1, v2) => v1.Year.CompareTo(v2.Year));
        return _vehicles;
    }
}

But to make things clear: You should highly consider, that overall idea of having that kind of property is wrong. You need your data to always be sorted? Use proper data structure, like SortedList<int, Vehicle> instead of standard List that is sorted every time property getter is being fired.

Upvotes: 2

Servy
Servy

Reputation: 203844

In the first case, you're returning a reference to the list, not a copy of the list. If the internal list is modified at a later point then the outer list will be able to observe those changes (because it's the same list after all).

When you use LINQ's OrderBy method from outside of the list it is going to defer execution. It won't be until you actually iterate the results of that sequence that it will go and look into the list; at which point is has most likely already been changed.

When you use ToList in the property getter you are taking a copy of the list, rather than just copying the reference to it, so it's basically a snapshot of that moment in time. At the time you get the property the list is [apparently] empty, but when it you iterate it it's [apparently] not.

If it's important to both have this ability to defer execution, and also that the items in this property always be accessed based on a sort not manually applied by the caller, then you can use something like this instead:

public IEnumerable<Vehicle> Vehicles
{
    get { return _vehicles.OrderBy(v => v.Year); }
}

That said, if it's important that the collection always be sorted when accessed it may be preferable to simply store the items in a sorted manor, so that accessing the items is cheap. Consider instead using something like a SortedSet instead of a List.

Upvotes: 4

Related Questions