10
\$\begingroup\$

I am looking to improve the following query.

We are using EF6 (CodeFirst) and C# Lambda Expressions, and Repository/Unit of Work patterns. Also, we needed to restrict the query in a way that we could inner-query the entities so that the query will only retrieve a subset of the joined data, and found this DynamicSelectExtensions library that extends IQueryable and gives us a SelectIncluding statement so that we could do this.

Query:

namespace Repositorios
{
    public class CanalDeComunicacionRepositorio: EFRepositorioDesactivable<CanalDeComunicacion>, ICanalDeComunicacionRepositorio
    {
        public CanalDeComunicacionRepositorio(DbContext context) : base(context) { }

        public IQueryable<CanalDeComunicacion> ConsultaParaLibreria()
        {
            return
                dbSet.Where(canalComunicacion=> canalComunicacion.FechaDesactivacion==null)
                .SelectIncluding(new List<Expression<Func<CanalDeComunicacion, object>>>()
                {
                    canalComunicacion => canalComunicacion.ListaControladores2.Where(controlador=>controlador.FechaDesactivacion==null),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaPuertas.Where(puerta=>puerta.FechaDesactivacion==null)),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaPuertas.Select( puerta=>puerta.ListaLectoras.Where(lectora=>lectora.FechaDesactivacion==null))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Where(lectora=>lectora.FechaDesactivacion==null)),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaEventosExcluidos.Where(eventos=> eventos.FechaDesactivacion==null)),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Where(grupoLectora=>grupoLectora.FechaDesactivacion==null && grupoLectora.GrupoAcceso.FechaDesactivacion==null))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGpoPadre.Where(gpoPadre => gpoPadre.FechaDesactivacion==null)))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGpoPadre.Select(gpoPadre => gpoPadre.GpoPadre)))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGpoPadre.Select(gpoPadre => gpoPadre.GpoPadre.ListaDeGrupoAccesoEmpleados.Where(listEmp => listEmp.FechaDesactivacion==null))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGpoPadre.Select(gpoPadre => gpoPadre.GpoPadre.ListaDeGrupoAccesoEmpleados.Select(listEmp => listEmp.Persona))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGpoPadre.Select(gpoPadre => gpoPadre.GpoPadre.ListaDeGrupoAccesoEmpleados.Select(listEmp => listEmp.Persona.ListaTarjetaPersona.Where(tarPer=>tarPer.FechaDesactivacion==null)))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGpoPadre.Select(gpoPadre => gpoPadre.GpoPadre.ListaDeGrupoAccesoEmpleados.Select(listEmp => listEmp.Persona.ListaTarjetaPersona.Select(tarPer=>tarPer.Tarjeta)))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGpoPadre.Select(gpoPadre => gpoPadre.GpoPadre.ListaDeGrupoAccesoEmpleados.Select(listEmp => listEmp.Persona.ListaTarjetaPersona.Select(tarPer=>tarPer.Tarjeta.ListaTiposTarjeta.Where(ltipo=>ltipo.FechaDesactivacion==null))))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeFeriados.Where(liferiado=> liferiado.FechaDesactivacion==null)))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGrupoAccesoEmpleados.Where(lisEmpleado=> lisEmpleado.FechaDesactivacion==null)))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeFeriados.Select(liferiado=> liferiado.Feriado)))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeHistorialTurnosRecurrencia.Where(turno=>turno.FechaDesactivacion==null)))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeHistorialTurnosRecurrencia.Select(turno=>turno.Turno).Select(turno=>turno)))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeHistorialTurnosRecurrencia.Select(turno=>turno.Turno).Select(turno=>turno.ListaDiasHora.Where(lista=>lista.FechaDesactivacion==null))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeHistorialTurnosRecurrencia.Select(turno=>turno.Turno).Select(turno=>turno.ListaDiasHora.Select(lista=>lista.Dia))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGrupoAccesoEmpleados.Select(lisEmpleado=> lisEmpleado.Persona)))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGrupoAccesoEmpleados.Select(grupoEmpleado=>grupoEmpleado.ListaDeHistorialTurnosRecurrenciaEspecial.Where(listaTurnoRecurrencia=> listaTurnoRecurrencia.FechaDesactivacion==null))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGrupoAccesoEmpleados.Select(grupoEmpleado=>grupoEmpleado.ListaDeHistorialTurnosRecurrenciaEspecial.Select(turno=>turno.Turno))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGrupoAccesoEmpleados.Select(grupoEmpleado=>grupoEmpleado.ListaDeHistorialTurnosRecurrenciaEspecial.Select(turno=>turno.Turno.ListaDiasHora.Where(lista=>lista.FechaDesactivacion==null)))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGrupoAccesoEmpleados.Select(grupoEmpleado=>grupoEmpleado.ListaDeHistorialTurnosRecurrenciaEspecial.Select(turno=>turno.Turno.ListaDiasHora.Select(lista=>lista.Dia)))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGrupoAccesoEmpleados.Select(lisEmpleado=> lisEmpleado.Persona.ListaTarjetaPersona.Where(lisTarjeta=>lisTarjeta.FechaDesactivacion==null))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGrupoAccesoEmpleados.Select(lisEmpleado=> lisEmpleado.Persona.ListaTarjetaPersona.Select(lisTarjeta=>lisTarjeta.Tarjeta))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaDeGruposAccesoLectora.Select(grupoAcceso=>grupoAcceso.GrupoAcceso).Select(grupo=>grupo.ListaDeGrupoAccesoEmpleados.Select(lisEmpleado=> lisEmpleado.Persona.ListaTarjetaPersona.Select(lisTarjeta=>lisTarjeta.Tarjeta.ListaTiposTarjeta.Where(list=>list.FechaDesactivacion==null)))))),
                    canalComunicacion => canalComunicacion.ListaControladores2.Select(controlador=>controlador.ListaLectoras.Select(lectoras=>lectoras.ListaTiposTarjeta.Where(list=>list.FechaDesactivacion==null)))
                }).AsQueryable();
        }
    }
}

The query just works as of right now, but it is a very slow query, and also the code is pretty ugly and long and nested.

Model:

This is the data model for the entities involved in the query. As you can see, we need to join a lot of tables and traverse the tree to get all the data we need.

Data Model

EDIT:

About the patterns we are using, they al are proposed by Julie Lerman and her recommendations for Entity Framework use in the enterprise. The solution layout includes these projects:

  • DomainClasses: all classes that are used in the data model.
  • DataLayer: DbContext is defined here, along with migrations, tables, special model configurations and seed data.
  • Repositorio and IRepositorio (Repositories): Here is all the logic that handles EF, in addition to UOW.

This structure helps us to have a single point of entry to the data, easing development and maintenance of our application.

Inside the Repositorio project, we have a generic repository (EFRepositorio) from which all of the specific repository implementations inherit:

namespace Repositorios
{
    public class EFRepositorio<T> : IRepositorio<T> where T : class
    {
        protected DbContext dbContext { get; set; }
        protected DbSet<T> dbSet { set; get; }

        public EFRepositorio(DbContext dbcontext)
        {
            if (dbcontext == null)
                throw new ArgumentNullException("dbContext es Nulo");
            dbContext = dbcontext;
            dbSet = dbContext.Set<T>();

        }

        public virtual IQueryable<T> DameTodos()
        {
            return dbSet;

        }

        public virtual T DamePorId(int id)
        {
            return dbSet.Find(id);
        }


        public virtual void Agregar(T entity)
        {
            DbEntityEntry dbEntityEntry = dbContext.Entry(entity);

            if (dbEntityEntry.State != EntityState.Detached)
            {
                dbEntityEntry.State = EntityState.Added;
            }
            else
            {
                dbSet.Add(entity);
            }
        }

        public virtual void AgregarActualizarGrafo(T entity)
        {
            dbSet.Add(entity);
            dbContext.ApplyStateChanges();
        }

        public virtual void Actualizar(T entity)
        {
            AgregarActualizarGrafo(entity);
        }

        public virtual void Borrar(T entity)
        {
            DbEntityEntry dbEntityEntry = dbContext.Entry(entity);
            if (dbEntityEntry.State != EntityState.Deleted)
            {
                dbEntityEntry.State = EntityState.Deleted;
            }
            else
            {
                dbSet.Attach(entity);
                dbSet.Remove(entity);
            }
        }

        public virtual void Borrar(int id)
        {
            var entity = DamePorId(id);

            if (entity == null) return;
            Borrar(entity);
        }
    }
}

Let me know if you need anything else.

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! You do realize that Entity Framework's DbContext is a unit-of-work, right? DbSet<T> is a repository, too... why make an abstraction of an abstraction? \$\endgroup\$ – Mathieu Guindon Jun 25 '14 at 17:49
  • \$\begingroup\$ Where is dbSet coming from? You might get a more thorough review if you also include the code for the base class you're deriving from. \$\endgroup\$ – Mathieu Guindon Jun 25 '14 at 17:52
  • 1
    \$\begingroup\$ Please check my edit!! I've included the requested information. Thanks! \$\endgroup\$ – beto_ve Jul 3 '14 at 0:23
  • \$\begingroup\$ Can you provide the sql table create script, or explain what you are trying to query, or the sql query that the above code is resulting in? I don't understand the language it's in so it's hard for me to understand what you are trying to accomplish with the query. \$\endgroup\$ – Erik Lieben Jul 5 '14 at 15:52
4
+50
\$\begingroup\$

You should replace some of your Selects for SelectMany. Normally IEnumerable extensions can be coded without nesting them, in your case if you replace your "grupoAcesso" selction by chaining SelectManys you could write this:

private IEnumerable<?> GetGrupoAcessoFromCanal(CanalDeComunicacion canal){
    return canal.ListaControladores2
        .SelectMany(controlador=>controlador.ListaLectoras)
        .SelectMany(lectoras=>lectoras.ListaDeGruposAccesoLectora)
        .SelectMany(grupoAcceso=>grupoAcceso.GrupoAcceso);
}

This is also a query that is repeated way too much and that's why I placed it in a method, now let just use it:

dbSet.Where(canalComunicacion=> canalComunicacion.FechaDesactivacion==null)
     .SelectIncluding(new List<Expression<Func<CanalDeComunicacion, object>>>()
     {
        //all queries that don't need our method.
        canalComunication => GetGrupoAcessoFromCanal(canalComunication),
        canalComunication => GetGrupoAcessoFromCanal(canalComunication).Select(grupo=>grupo.ListaDeGpoPadre.Where(gpoPadre => gpoPadre.FechaDesactivacion==null)),
        //so on so forth...
     }

You should also apply this concept for all other nested queries that you have. Having your queries written in this way makes your code way more readable and natural.

\$\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.