Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C#: Update Item value in List<of T>

Tags:

c#

list

linq

I'm looking for a way to update an Element in a List without enumerating it on my own.

I got the Class MyProjects which hold a List named Projects.

I want to find the MyProjects.Projects-Class, where a member property of Class1 (Name) equals the Value "Overhead".

What works:

foreach (Project prj in MyProjects.Projects) {
    if (prj.Name == "Overhead")
        prj.IsActive = true;
}; 

I, however, try to do the same by using Linq, but failed in writing it as one line. Is this even possible? The reason why I don't like to iterate in the way above is that I already iterate the whole list in this codeblock and think, that there might be a more beautiful way :)

like image 748
AllDayPiano Avatar asked Oct 07 '15 13:10

AllDayPiano


Video Answer


2 Answers

You shouldn't try to get everything down to one line - just as brief as is readable. In this case, you can use:

foreach (var project in MyProjects.Projects.Where(p => p.Name == "Overhead"))
{
    project.IsActive = true;
}

That's using LINQ for the querying part, which is appropriate as that's what the Q of LINQ stands for. I'd strongly urge you not to mutate items within LINQ calls in the way that Mayank's answer does though. It's error-prone (as evidenced by the original answer not working) and against the spirit of LINQ.

That's about as readable as it gets, IMO. It does exactly the same thing as the original code, mind you - you can't avoid something iterating over every item in the list, if every item might be one you want to update.

EDIT: Just for laughs, if you really, really wanted to do it in pretty minimal code, you could use:

// DON'T USE THIS!
MyProjects.Project.Count(p => p.Name == "Overhead" && (p.IsActive = true));

Here we use the fact that && is short-circuiting to avoid evaluating the assignment (p.IsActive = true) unless the condition is matched. It's handy that we're assigning a bool value to a property, as that means we don't need to do anything else to make it a valid second operand for the && operator. We use Count() to fully evaluate the result without creating any additional lists etc - and we use the version with a predicate to avoid even needing a Where call, which a previous version did. (LastOrDefault would work too.) But it's all a horrible abuse, and should never appear in any real code.

like image 117
Jon Skeet Avatar answered Oct 06 '22 05:10

Jon Skeet


I've come up with a way to get it down to one line, without abusing LINQ, since I'm only using it for the querying part (filter), and using a custom extension method to perform the property setting action. You're still going to enumerate the items (you have to) but you can hide that away in the extension method. I suspect that you didn't really care whether you enumerated the item or not, you just didn't like the amount of visible space a foreach loop would take up in your main code.

Use this extension method:

public static IEnumerable<T> SetProperty<T>(this IEnumerable<T> list, Action<T> action)
{
    foreach (var item in list)
    {
        action.Invoke(item);
    }
    return list;
}

This allows you to get it down to one readable line.

Projects.Where(p => p.Name == "Overhead").SetProperty(p => p.IsActive = true);

Complete test program:

using System;
using System.Collections.Generic;
using System.Linq;

namespace ConsoleApplication2
{
    class Program
    {
        static void Main(string[] args)
        {
            var Projects = new List<Project>() {
                new Project() { Name="Overhead", IsActive=false },
                new Project() { Name="Nadfadfs", IsActive=false },
                new Project() { Name="Overhead", IsActive=false },
                new Project() { Name="dasfasdf", IsActive=false }
            };
            PrintProjectList(Projects);
            Console.WriteLine("--Setting property--");
            Projects.Where(p => p.Name == "Overhead").SetProperty(p => p.IsActive = true);
            PrintProjectList(Projects);
            Console.WriteLine("Press any key to exit.");
            Console.ReadKey();
        }

        static void PrintProjectList(IEnumerable<Project> projects)
        {
            foreach(var p in projects)
            {
                Console.WriteLine($"Name: {p.Name} IsActive: {p.IsActive}");
            }
        }
    }

    class Project
    {
        public string Name { get; set; }
        public bool IsActive { get; set; }
    }

    public static class Extensions
    {
        public static IEnumerable<T> SetProperty<T>(this IEnumerable<T> list, Action<T> action)
        {
            foreach (var item in list)
            {
                action.Invoke(item);
            }
            return list;
        }
    }
}

Output:

Name: Overhead IsActive: False

Name: Nadfadfs IsActive: False

Name: Overhead IsActive: False

Name: dasfasdf IsActive: False

--Setting Property--

Name: Overhead IsActive: True

Name: Overhead IsActive: False

Name: Overhead IsActive: True

Name: Overhead IsActive: False


It turns out that my SetProperty function is very similar to the ForEach that's already built into the framework. The main difference being that mine can operate on any IEnumerable<T>. That syntax is loved by some, and hated by others, for reasons that Eric Lippert pointed out on his blog (Thanks to Jon Skeet for pointing this out). Also, see this discussion by the Microsoft team. I'll leave it to you to draw your own conclusion.

On a side note, calling it SetProperty is kind of inaccurate, because you could do any action on the items in the collection. You could call it ForEach, but that clashes with the framework. Not positive what I'd call it, but perhaps PerformAction.

like image 45
mason Avatar answered Oct 06 '22 06:10

mason