2
\$\begingroup\$

The following code fetches data from a MySql database to generate a simple report on accrued work time on projects.

I've been away from Entity Framework for a while and looking at this code you can see that the creation of ListProject is basically repeated 3 times.

Is there any way to refactor this so I can reuse the duplicate parts?

I've thought about a factory method, but that won't work because it can't be translated to SQL by the expression interpreter.

    var project = entities.Tickets
        .Where(p => p.ID == id)
        .Select(p => new ListProject
        {
            Id = p.ID,
            Name = p.ueberschrift,
            AssignedEmployee = new ListEmployee
            {
                Id = p.AssignedEmployee.ID,
                Name = p.AssignedEmployee.name
            },
            WorkUnits = p.WorkUnits
                .Select(wu => new ListProjectWorkUnit
                {
                    Duration = wu.dauer,
                    PauseDuration = wu.pauseDauer,
                    Employee = new ListEmployee
                    {
                        Id = wu.AssignedEmployee.ID,
                        Name = wu.AssignedEmployee.name
                    }
                })
                .ToList()
            })
            .Single();
    project.Tickets = entities.Tickets
        .Where(p => p.vonProjekt == id)
        .Where(p => p.phaseID == 0)
        .OrderBy(p => p.ueberschrift)
        .Select(p => new ListProject
        {
            Id = p.ID,
            Name = p.ueberschrift,
            AssignedEmployee = new ListEmployee
            {
                Id = p.AssignedEmployee.ID,
                Name = p.AssignedEmployee.name
            },
            WorkUnits = p.WorkUnits
                .Select(wu => new ListProjectWorkUnit
                {
                    Duration = wu.dauer,
                    PauseDuration = wu.pauseDauer,
                    Employee = new ListEmployee
                    {
                        Id = wu.AssignedEmployee.ID,
                        Name = wu.AssignedEmployee.name
                    }
                })
                .ToList()
        })
        .ToList();
    project.Phases = entities.projekt_phase
        .Where(p => p.projektID == id)
        .OrderBy(p => p.rang)
        .Select(p => new ListProjectPhase
        {
            Id = p.ID,
            Name = p.name,
            Tickets = p.Tickets
                .OrderBy(t => t.ueberschrift)
                .Select(t => new ListProject
                {
                    Name = t.ueberschrift,
                    Id = t.ID,
                    AssignedEmployee = new ListEmployee
                    {
                        Id = t.AssignedEmployee.ID,
                        Name = t.AssignedEmployee.name
                    },
                    WorkUnits = t.WorkUnits
                        .Select(wu => new ListProjectWorkUnit
                        {
                            Duration = wu.dauer,
                            PauseDuration = wu.pauseDauer,
                            Employee = new ListEmployee
                            {
                                Id = wu.AssignedEmployee.ID,
                                Name = wu.AssignedEmployee.name
                            }
                        })
                        .ToList()
                })
                .ToList()
        })
        .ToList();
\$\endgroup\$
  • \$\begingroup\$ Please fix your code indentation, it would help with the review :) \$\endgroup\$ – IEatBagels May 24 at 15:11
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Mast May 24 at 15:19
  • \$\begingroup\$ @Mast I understand, but I tried to correct a copy pasta error. One of those .Single()s is a .ToList() in production code. \$\endgroup\$ – w4n May 24 at 15:22
  • \$\begingroup\$ Well, next time do it right the first time and please take a look at the help center. We take answer invalidation quite seriously here and people reviewing different versions of the same code in the same question gets very messy very fast. \$\endgroup\$ – Mast May 24 at 15:27
1
\$\begingroup\$

I've thought about a factory method, but that won't work because it can't be translated to SQL by the expression interpreter.

Why don't you call (as suggested by t3chb0t) AsEnumerable() right before your factory method? You would perform the factory method in memory.

 var project = entities.Tickets
        .Where(p => p.ID == id)
        .AsEnumerable()   // <-- loads the tickets into memory
        .Select(p => myFactoryMethod(p))
        .Single();
\$\endgroup\$
  • \$\begingroup\$ Thanks, I've thought about that, but I would need a bunch of .Includes to get all the nested information I need. Plus I don't want to have all the content of these tables in memory as they're a bit meaty. \$\endgroup\$ – w4n May 24 at 15:12
  • \$\begingroup\$ @w4n then have you thought about making a view and mapping this with EF? \$\endgroup\$ – dfhwze May 24 at 15:14
  • \$\begingroup\$ Unfortunately, this isn't our data base. This is a ticketing system to which we only have read access. \$\endgroup\$ – w4n May 24 at 15:18
  • \$\begingroup\$ @w4n I remember back in the day that we also had some boiler-plate code with Includes. But we managed to put this in reusable code blocks deep in our repositories. I don't have any examples of this code available anymore. Do you think when going the Include path, you still need to duplicate your code? \$\endgroup\$ – dfhwze May 24 at 15:20
  • \$\begingroup\$ I think the preferred way is to use the .AsEnumerable() extension instead of ToList, I'm not sure but my gut tells me that it would then loop over the result only once. At least this is what I'm always doing... \$\endgroup\$ – t3chb0t May 24 at 15:21
0
\$\begingroup\$

I can't check this with a compiler right now but general idea is that you can benefit from IQueryable. Define method like this

IQueryable<ListProject> SelectListProjects(IQueryable<Tickets> tickets) 
{
    return entities.Tickets
        .Where(p => p.vonProjekt == id)
        .Where(p => p.phaseID == 0)
        .OrderBy(p => p.ueberschrift)
        .Select(p => new ListProject
        {
            Id = p.ID,
            Name = p.ueberschrift,
            AssignedEmployee = new ListEmployee
            {
                Id = p.AssignedEmployee.ID,
                Name = p.AssignedEmployee.name
            },
            WorkUnits = p.WorkUnits
                .Select(wu => new ListProjectWorkUnit
                {
                    Duration = wu.dauer,
                    PauseDuration = wu.pauseDauer,
                    Employee = new ListEmployee
                    {
                        Id = wu.AssignedEmployee.ID,
                        Name = wu.AssignedEmployee.name
                    }
                })
                .ToList()
            });            
}

and use it as follows

var project = SelectListProjects(entities.Tickets
        .Where(p => p.ID == id)).Single();
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.