Convert Foreach into linq Expression-CodePudding

I am trying to get list of devices against a current customer from a Device table. I did it but with a foreach loop, but I wanted to do it operation using linq, instead.

here is my class structure

public class Device
{
    public string type { get; set; }
    public int id { get; set; }
    public List<Role> roles { get; set; }
}


public class Role
{
    public int roleId { get; set; }
    public string roleName { get; set; }
    public int customerId { get; set; }
}

Here is my Code

var devList = list.Where(x => x.type == "device1").ToList();

foreach (var item in devList)
{
    if (item.roles != null)
    {
        var kss = item.roles.Where(z => z.customerId == kunId).FirstOrDefault();
        if (kss != null)
        {
            m.devicesList.Add(item);
        }
    }
}

I need to get devices against the current customer, so I have to compare my current user id with customerId.

How can I convert this to linq-to-sql?

CodePudding user response：

There are two ways to approach this.

First, you could put everything into a complicated Where operation:

var devices = list.Where(d => d.type == "device1" && d.roles is object && d.roles.Any(r => r.customerId == kunId) );

Second, you could spread it out over multiple separate operations, but each operation is much easier to understand:

var devices = list.Where(d => d.type == "device1").
                   Where(d => d.roles is object).
                   Where(d => d.roles.Any(r => r.customerId == kunId) );

The performance will be similar for either option. There is overhead for the additional Where() calls, but not as much as you might think... the JITter can do some amazing things with this kind of code, and I wouldn't be surprised to see both of these end up with the exact same IL.

Also notice in both cases there is no ToList() call. You can very often greatly improve performance and memory use by sticking with IEnumerable for longer.

At this point, it's unclear to me from the question whether you are merely appending into the existing m.devicesList or whether these values can replace whatever might currently be in that collection.

Assuming m.devicesList is declared as List<Device> (or similar), the former option would look like this:

m.devicesList.AddRange(devices);

Again, there was no need to ever call ToList().

The latter would look like this:

m.devicesList = devices.ToList();

We could also combine both steps into a single statement. Here's one of the four possible combinations:

m.devicesList.AddRange(list.Where(d => d.type == "device1").
                            Where(d => d.roles is object).
                            Where(d => d.roles.Any(r => r.customerId == kunId) )
                      );

CodePudding user response：

Always try to "implement" your requirements in plain natural language first.

What devices do you need? That ones which:

have type of "device1"
have role assigned with given customer

Construct each part separately, add necessary guards e.g. roles != null and you get something like this:

var devicesFound = list.Where(d => 
   d.type == "device1" && 
   d.roles != null && 
   d.roles.Any(r => r.customerId == kunId));

m.devicesList.AddRange(devicesFound);

CodePudding user response：

You should be able to remove the .ToList() at the end of the linq expression value you're assigning to devList.

Additionally, if you're aiming to optimize the operation you're performing, you might consider maintaining the current code, instead of converting it to a linq expression.

Still, if you need to convert the foreach loop, have a look at the official microsoft documentation, you should be able to do it automatically with the help of intellisense.