C# Optimizing the Code

Question

I have the following function which returns a Model. It is taking time say if there are 2000 employees, it takes 3-4 minutes to return data. I actually want to optimize this function. I have done few things which are included in the below code, but still it takes lots of time.

using (var ctx = new ApplicationDbContext(schemaName))
{
                List<Employee> list = new List<Employee>();
                Employee mod = new Employee();

                var data = ctx.Employee.Where(a => a.Company == comp && a.Status == Live)
                    .Select(a => new
                    {
                        Id = a.Id,
                        Code = a.Code,
                        FName = a.FName,
                        DateOfJoining = a.DateOfJoining,
                        Category = a.Category,
                        Department = a.Department,
                        Designation = a.Designation;
                    })
                    .ToList();

                var loadValues = ctx.CValue.Where(c => c.Company == comp).ToList();

                foreach (var item in data)
                {
                    mod = new Employee();

                    mod.Id = item.Id;
                    mod.Code = item.Code;
                    mod.FName = item.FName;
                    mod.DateOfJoining = item.DateOfJoining;
                    mod.Category = item.Category;
                    mod.Designation = item.Designation;
                    mod.Department = item.Department;

                    int designation = (item.Designation == null) ? 0 : item.Designation;
                    int department = (item.Department == null) ? 0 : item.Department;

                    if (designation != 0)
                        mod.DesignationString = loadValues.Where(c => c.CompanyId == comp && c.Id == designation).Select(c => c.ComboValue).FirstOrDefault();
                    if (department != 0)
                        mod.DepartmentString = loadValues.Where(c => c.Company == comp && c.Id == department).Select(c => c.ComboValue).FirstOrDefault();

                    list.Add(mod);
                }

                return list;
            }
}

I think it is the foreach loop which is taking time. Any workaround for this?How to optimize the above Code?

How is ctx populated? The code you have provided shouldn't take that long — Joakim Hansson, Jul 05 '16 at 10:58
How about measuring the time using Stopwatch, line by line ? — , Jul 05 '16 at 10:59
_"I think it is the `foreach` loop which is taking time"_ Find out for sure. There are good tools for this sort of thing, for example (random pick): [ANTS](http://www.red-gate.com/products/dotnet-development/ants-performance-profiler/). You need to know which bit is slow before you can start optimising it. — James Thorpe, Jul 05 '16 at 11:00
If it is indeed the ForEach you could alway use a parralell foreach since the list is only added to — knechtrootrecht, Jul 05 '16 at 11:01
`Why do you` format certain phrases `as code` when they are `not actually code`? — Cody Gray, Jul 05 '16 at 11:03
What actually is taking the time is the linq WHERE. The linq produces an enumeration. Each time you go through the for loop the next item in the Linq Where is found. It is taking a 1/4 second for each employee (4 minutes / 2000)which isn't too bad especially if you are getting the data from a database. — jdweng, Jul 05 '16 at 11:05
Maybe try to find your company first. Since you are using Linq, you can use `companyInstance.Employees`. That will be executed as join which is faster. And I believe you don't need to have `var data` as `List<...>`, `IQueryable<>` is sufficient and you will awoid obtaining whole list and thern recreating new list with almost same values — Pavel Pája Halbich, Jul 05 '16 at 11:06
This code wouldn't even compile. Could you post your actual code that compiles? — Enigmativity, Jul 05 '16 at 11:07

Zein Makki · Answer 1 · 2016-07-05T11:20:56.320

You can optimze the nested loops ( Linq's Where inside the foreach) by using a fast lookup data structure:

var loadValues = (from c in ctx.CValues
                    where c.Company == comp
                    select c).ToLookup(x => Tuple.Create(x.CompanyId, x.ID), 
                                        x => x.ComboValue);

foreach (var item in data)
{
    // your same code goes here 
    // then

   if (designation != 0)
      mod.DesignationString = loadValues[Tuple.Create(comp, designation)].FirstOrDefault();
   if (department != 0)
      mod.DepartmentString = loadValues[Tuple.Create(comp, department)].FirstOrDefault();

    list.Add(mod);
}

I can't see any other part that can be optimized.

It is worth noting the filter on comp is redundant inside the if block as it is already part of your initial query.

score 2 · Answer 2 · answered Jul 05 '16 at 11:15

I've optimized code into one linq query. i think it will be faster

using (var ctx = new ApplicationDbContext(schemaName))
{
    var loadValues = ctx.CValue.Where(c => c.Company == comp).ToList();
    return ctx
        .Employee
        .Where(a => a.Company == comp && a.Status == Live)
        .Select(item => new Employee
        {
            Id = item.Id,
            Code = item.Code,
            FName = item.FName,
            DateOfJoining = item.DateOfJoining,
            Category = item.Category,
            Designation = item.Designation,
            Department = item.Department,
            DesignationString = loadValues.Where(c => c.CompanyId == comp && c.Id == item.Designation ?? 0).FirstOrDefault(c => c.ComboValue);
            DepartmentString = loadValues.Where(c => c.Company == comp && c.Id == item.Department ?? 0).FirstOrDefault(c => c.ComboValue);
        });
}

This does pretty much the same subquery on a sequential List, so I don't think there is much difference. It *might* be slightly faster, but there is no substantial gain. — derpirscher, Jul 05 '16 at 11:30
I get the error `The entity or complex type 'Models.Employee' cannot be constructed in a LINQ to Entities query.` — Anup, Jul 05 '16 at 12:37

derpirscher · Answer 3 · 2016-07-05T11:25:28.187

The following snipped might take quite long (do a performance measurement, to be sure)

var loadValues = ctx.CValue.Where(c => c.Company == comp).ToList();

....

if (designation != 0) mod.DesignationString = loadValues.Where(c => c.CompanyId == comp && c.Id == designation).Select(c => c.ComboValue).FirstOrDefault();
if (department != 0) mod.DepartmentString = loadValues.Where(c => c.Company == comp && c.Id == department).Select(c => c.ComboValue).FirstOrDefault();

because loadValues is a List which is always searched sequentially. So depending on the size of loadValues there may be a lot of searches in this list. Furthermore, you don't need to compare for CompanyId because you already filtered by CompanyId in the definition of loadValues.

To speed up things here, you could use a Lookup.

var loadValues = ctx.CValue.Where(c => c.Company == comp).ToLookup(x=> x.Id);

if (designation != 0 && loadValues.Contains(designation)) mod.DesigationString = loadValues[designation].Select(c => c.ComboValue).FirstOrDefault();
if (department != 0 && loadValues.Contains(department)) mod.DepartmentString = loadValues[department].Select(c => c.ComboValue).FirstOrDefault();

Or, as you are searching by Id which should be unique you could also make a simple Dictionary<int, string>.

var loadValues = ctx.CValue.Where(c=>c.Company == comp).ToDictionary(x=> x.Id, y=> y.ComboValue);

 if (designation != 0) mod.DesigationString = loadValues.ContainsKey(designation) ? loadValues[designation] : String.Empty;
 if (department != 0) mod.DepartmentString = loadValues.ContainsKey(department) ? loadValues[department] : String.Empty;

score 2 · Answer 4 · edited May 23 '17 at 12:10

I'd write it that way:

using (var ctx = new ApplicationDbContext(schemaName))
{
    var company = ctx.Companies.FirstOrDefault(e => e.ID == 42);
    if(company == null)
        throw new Exception();

    var empl = company.Employees.Where(e=> e.Status == Live).Select(e=>
        new EmployeeInfo{
            ID = e.ID,
            FName = e.FName,
            //TODO
            DesignationString = e.Designation != null ? e.Designation.ComboValue : string.Empty,
            //TODO

        });

    return empl.ToList();
}

But I assume that you have DB structure with proper Foreign keys and Designation is column referencing table CValue. There is no nested loops, no doubled memory allocations (as I mentioned in my comment) and everything will be obtained using Join, which is way more faster than Where clausule.

EDIT You need to return List of models different than mapped entity. Look at the line 8 of my example. If you try to run .Select(e => new Employee()), you will get error you mentioned in comments. (More info here: The entity cannot be constructed in a LINQ to Entities query.)

But if you return List<EmployeeInfo>, you can work anywhere in upper layers of your application with that list, you can add more elements (and they can be result of other methods than DB query, for example XML import) and you will be independent to actual DB mapped class.

You can find more info about app composition here: https://softwareengineering.stackexchange.com/questions/240704/confused-about-layered-application-development.

I get the error The entity or complex type 'Models.Employee' cannot be constructed in a LINQ to Entities query. — Anup, Jul 06 '16 at 04:35

C# Optimizing the Code

4 Answers4