views:

119

answers:

1

I feel like this is not a very efficient way of using linq. I was hoping somebody on here would have a suggestion for a refactor. I realize this code is not very pretty, as I was in a complete rush.

public class Workflow
{

    public void AssignForms()
    {

        using (var cntx = new ProjectBusiness.Providers.ProjectDataContext())
        {
            var emplist = (from e in cntx.vw_EmployeeTaskLists where e.OwnerEmployeeID == null select e).ToList();
            foreach (var emp in emplist)
            {
                // if employee has a form assigned: break;

                if (emp.GRADE > 15 || (emp.Pay_Plan.ToLower().Contains("al") || emp.Pay_Plan.ToLower().Contains("ex")))
                {
                    //Assign278();
                }
                else if ((emp.Series.Contains("0905") || emp.Series.Contains("0511") || emp.Series.Contains("0110") || emp.Series.Contains("1801")) || (emp.GRADE >= 12 && emp.GRADE <= 15))
                {
                    var emptask = new ProjectBusiness.Providers.EmployeeTask();
                    emptask.TimespanID = cntx.Timespans.SingleOrDefault(t => t.BeginDate.Year == DateTime.Today.Year & t.EndDate.Year == DateTime.Today.Year).TimespanID;
                    var FormID = (from f in cntx.Forms where f.FormName.Contains("450") select f.FormID).FirstOrDefault();
                    var TaskStatusID = (from s in cntx.TaskStatus where s.StatusDescription.ToLower() == "not started" select s.TaskStatusID).FirstOrDefault();
                    Assign450((int)emp.EmployeeID, FormID, TaskStatusID, emptask);
                    cntx.EmployeeTasks.InsertOnSubmit(emptask);
                }
                else
                {
                    //Assign185();
                }

            }
            cntx.SubmitChanges();
        }

    }

    private void Assign450(int EmployeeID, int FormID, int TaskStatusID, ProjectBusiness.Providers.EmployeeTask emptask)
    {
        emptask.FormID = FormID;
        emptask.OwnerEmployeeID = EmployeeID;
        emptask.AssignedToEmployeeID = EmployeeID;
        emptask.TaskStatusID = TaskStatusID;
        emptask.DueDate = DateTime.Today;
    }
}
+2  A: 

If you are using a LINQ provider try to use queries and not loops.

public void AssignForms()
{

    using (var cntx = new ProjectDataContext())
    {
        var emplist = from e in cntx.vw_EmployeeTaskLists
                      where e.OwnerEmployeeID == null
                      select e;

        var timeSpanId = cntx.Timespans.SingleOrDefault(t =>
                            t.BeginDate.Year == DateTime.Today.Year &&
                            t.EndDate.Year == DateTime.Today.Year
                            ).TimespanID;

        var formID = (from f in cntx.Forms
                      where f.FormName.Contains("450")
                      select f.FormID).FirstOrDefault();

        var taskStatusID = (from s in cntx.TaskStatus
                            let status = s.StatusDescription.ToLower()
                            where status == "not started"
                            select s.TaskStatusID).FirstOrDefault();

        var assign278 = from emp in emplist
                        let payPlanInv = emp.Pay_Plan.ToUpperInvariant()
                        where emp.GRADE > 15 || (
                              payPlanInv.Contains("AL") ||
                              payPlanInv.Contains("EX")
                              )
                        select emp;

        var assign450 = from emp in emplist
                        where (emp.Series.Contains("0905") ||
                               emp.Series.Contains("0511") ||
                               emp.Series.Contains("0110") ||
                               emp.Series.Contains("1801")
                              ) || (
                               emp.GRADE >= 12 &&
                               emp.GRADE <= 15)
                        select emp;

        var assign185 = from emp in emplist
                        where !assign278.Select(e => e.EmployeeID)
                                        .Contains(emp.EmployeeID)
                              && !assign450.Select(e => e.EmployeeID)
                                           .Contains(emp.EmployeeID)
                        select emp;

        // do inserts here
        cntx.EmployeeTasks.InsertAllOnSubmit(assign450.Select(emp =>
                     new EmployeeTask()
                     {
                         TimespanID = timeSpanId,
                         FormID = formID,
                         OwnerEmployeeID = emp.EmployeeID,
                         AssignedToEmployeeID = emp.EmployeeID,
                         TaskStatusID = taskStatusID,
                         DueDate = DateTime.Today
                     }));
        cntx.SubmitChanges();
    }
}
Matthew Whited
This was a fast best guess... it may require some work to get the way you want.
Matthew Whited
@Matthew Whited: This makes sense. It's much more efficient. It worked almost perfectly... only had to tweak a little.
Russell Boland
Excellent... glad I could help. Changing those `.Contains(...)` to `==` will help the query plan for the server (should change from `LIKE` to `=`.) If you are using LINQ2SQL and would like to see the query that is send to the server call the `.ToString()` method on the query. This may help you better understand what LINQ is doing for you.
Matthew Whited