Pop Linq Extension

Topics: C# Language Design
Nov 18, 2014 at 10:37 PM
Edited Nov 18, 2014 at 10:40 PM
When we are iterating over a list we sometimes like to remove the items which have already been iterated over. The Pop Extension is something we used in recent code to do just that in linq format.

I use the .IsEmpty Extension a coworker wrote to verify that a list is not empty.

Not sure if this would qualify as something that could be added to C# 6.0 or if it is something that would be of interest to anyone else.
  public static List<T> Pop<T>(this List<T> source, Func<T, bool> predicate) 
        {
            if (source == null) throw new ArgumentNullException("source");
            if (predicate == null) throw new ArgumentNullException("predicate");

            var results = source.Where(predicate).ToList();
            if ( results.IsEmpty() )
            {
                return null;
            }
            foreach (var result in results)
            {
                source.Remove(result);
            }
            return results;

        }
There is probably a more efficient way to remove the subset of results from the source list without iterating over the entire list.
Nov 18, 2014 at 11:58 PM
dcumin39 wrote:
When we are iterating over a list we sometimes like to remove the items which have already been iterated over. The Pop Extension is something we used in recent code to do just that in linq format.

I use the .IsEmpty Extension a coworker wrote to verify that a list is not empty.

Not sure if this would qualify as something that could be added to C# 6.0 or if it is something that would be of interest to anyone else.
  public static List<T> Pop<T>(this List<T> source, Func<T, bool> predicate) 
        {
            if (source == null) throw new ArgumentNullException("source");
            if (predicate == null) throw new ArgumentNullException("predicate");

            var results = source.Where(predicate).ToList();
            if ( results.IsEmpty() )
            {
                return null;
            }
            foreach (var result in results)
            {
                source.Remove(result);
            }
            return results;

        }
There is probably a more efficient way to remove the subset of results from the source list without iterating over the entire list.
That would qualify as a framework enhancement, not a language enhancement. The language teams can only really work with the types and methods provided by the frameworks.

There are already two methods of accomplishing this with the framework. If you want to modify the existing List<T> you can use the instance method RemoveAll. If you want to produce a new list only containing the members that match the predicate you can chain Where and ToList.
Nov 19, 2014 at 2:16 AM
Got it. I will have to resubmit this when there is a change to the framework.

I did update it to use RemoveAll as you suggested

        public static List<T> PopWhere<T>(this List<T> source, Func<T, bool> predicate) 
        {
            if (source == null) throw new ArgumentNullException("source");
            if (predicate == null) throw new ArgumentNullException("predicate");

            var results = source.Where(predicate).ToList();
            if ( results.IsEmpty() )
            {
                return null;
            }
   
            source.RemoveAll(results.Contains);

            return results;
        }
Example usage of this would be
            var colors = new List<string>(){"red","blue","purple"};
            var flowers = new Dictionary<string, string> {{"roses", "red"},{"violets","blue"}};


            foreach ( var flower in flowers )
            {
                    var currentColor = colors.PopWhere(p => p == flower.Value);    
                    Console.WriteLine("{0} are {1}", flower.Key, currentColor.FirstOrDefault());       
            }
            colors.ForEach(p=>Console.WriteLine("No flowers are {0}",p));
Which would output
roses are red
violets are blue
No flowers are purple
Nov 19, 2014 at 4:44 PM
Edited Nov 19, 2014 at 4:46 PM
dcumin39 wrote:
if ( results.IsEmpty() )
{
   return null;
}
Please don't do this. If you want to return an empty collection, return an empty collection, not null. This approach leads to unnecessary NullReferenceExceptions and doesn't improve anything in return.

Even your own example usage has exactly this bug. You even use FirstOrDefault(), which would work correctly with an empty collection, but will throw for null.
Nov 20, 2014 at 9:03 PM
This implementation is O(n^2), and may really bog down for big lists; particularly if you are removing elements towards the end of the list.

I think the only place where this is a good use case is if some object containing the target list will not let you assign a new list, and only modify the existing list; but if we can set a new instance of the list in there, it will be always be more performant to just do source.Where(predicate).ToList(), since that work is already done as the first step.

That use case is certainly one that exists, but it seems like a pretty cornerey case and potentially inefficient implementation for an in-framework extension.

The implementation should be quite different for a linked list than an indexed list as well, which means that a in-framework extension really would want to target List<T> instead of IList<T>, which seems to me to also make it less general.
Nov 20, 2014 at 9:55 PM
While it's great that you want to contribute code, I would vote against something like this being in the framework for a variety of reasons:
  • It's O(n^2).
  • It returns null when the predicate matches nothing rather than an empty list.
  • There's no documentation.
  • The naming doesn't make sense. This isn't a stack and you're not popping anything. This method is closer to RemoveWhere than Pop.
  • The use-case for it is not great. The trend in programming is towards using immutable data structures and methods. None of the LINQ methods you are familiar with mutate the state of the underlying data structure; they either apply filters on top or create new data structures. The use-cases for when you need to mutate a list in this manner should only be in extremely memory or performance sensitive code. And you don't want to encourage poor practices by baking methods into the framework which make those practice easier.
Nov 25, 2014 at 5:32 AM
I'm not sure that BCL team will be accepting contributions of that sort. However there is a library to comlement LINQ: https://www.nuget.org/packages/morelinq/ ( https://code.google.com/p/morelinq/ )

I think, some patches there would be welcome.
Nov 25, 2014 at 1:33 PM
kekekeks wrote:
I'm not sure that BCL team will be accepting contributions of that sort. However there is a library to comlement LINQ: https://www.nuget.org/packages/morelinq/ ( https://code.google.com/p/morelinq/ )

I think, some patches there would be welcome.
I would be curious as to how amenable the .NET Core team will be to pull requests. I'm sure that they will have an exceptionally high standard but the other teams (EF, ASP.NET) do accept community contributions.