2010-06-03 10 views
8

Estoy intentando descubrir cómo refactorizar este código LINQ. Este código y otro código similar se repite dentro del mismo archivo y en otros archivos. En algún momento, los datos que se manipulan son idénticos y, a veces, los datos cambian y la lógica sigue siendo la misma.Cómo refactorizar este código LINQ duplicado?

Aquí hay un ejemplo de lógica duplicada que opera en diferentes campos de diferentes objetos.

public IEnumerable<FooDataItem> GetDataItemsByColor(IEnumerable<BarDto> dtos) 
{ 
    double totalNumber = dtos.Where(x => x.Color != null).Sum(p => p.Number); 
    return from stat in dtos 
      where stat.Color != null 
      group stat by stat.Color into gr 
      orderby gr.Sum(p => p.Number) descending 
      select new FooDataItem 
      { 
       Color = gr.Key, 
       NumberTotal = gr.Sum(p => p.Number), 
       NumberPercentage = gr.Sum(p => p.Number)/totalNumber 
      }; 
} 

public IEnumerable<FooDataItem> GetDataItemsByName(IEnumerable<BarDto> dtos) 
{ 
    double totalData = dtos.Where(x => x.Name != null).Sum(v => v.Data); 
    return from stat in dtos 
      where stat.Name != null 
      group stat by stat.Name into gr 
      orderby gr.Sum(v => v.Data) descending 
      select new FooDataItem 
      { 
       Name = gr.Key, 
       DataTotal = gr.Sum(v => v.Data), 
       DataPercentage = gr.Sum(v => v.Data)/totalData 
      }; 
} 

¿Alguien tiene una buena manera de refactorizar esto?

+0

¿Los nombres de las propiedades en 'FooDataItem' tienen que ser diferentes? La solución sería más simple si fueran más genéricos, p. 'Key',' Total', 'Porcentage'. –

+0

+1 Si esta es una representación exacta, sus métodos son pequeños y hacen exactamente lo que cabría esperar. ¿Quizás haya otras partes de tu código que se beneficiarían de la refactorización primero? –

Respuesta

10

Algo como esto:

public IEnumerable<FooDataItem> GetDataItems<T>(IEnumerable<BarDto> dtos, 
    Func<BarDto, T> groupCriteria, 
    Func<BarDto, double> dataSelector, 
    Func<T, double, double, FooDataItem> resultFactory) 
{ 
    var validDtos = dtos.Where(d => groupCriteria(d) != null); 
    double totalNumber = validDtos.Sum(dataSelector); 

    return validDtos 
     .GroupBy(groupCriteria) 
     .OrderBy(g => g.Sum(dataSelector)) 
     .Select(gr => resultFactory(gr.Key, 
            gr.Sum(dataSelector), 
            gr.Sum(dataSelector)/totalNumber)); 
} 

En su ejemplo, que podríamos llamar de esta manera:

GetDataItems(
    x => x.Color, // the grouping criterion 
    x => x.Number, // the value criterion 
    (key, total, pct) => 
     new FooDataItem { 
      Color = key, NumberTotal = total, NumberPercentage = pct }); 

Si ha cambiado FooDataItem a ser más genérico, que sería más fácil.

+1

este es un hermoso código. – Femaref

+1

Agradable. Para facilitar la lectura y evitar tener que refactorizar todas las llamadas a funciones existentes, probablemente aún ajuste su llamada a 'GetDataItems' en una función' GetDataItemsByColor (IEnumerable dtos) '. – Jelly

1

Creo que si refactorizas esto, sería más difícil de leer que lo que ya tienes. Cualquier cosa que se me ocurra involucra a Linq dinámico, o modificando o encapsulando BarDto para tener algún tipo de elemento especializado para ser utilizado únicamente para la agrupación.

3

No usaría la sintaxis de la consulta para esto, use la cadena de métodos.

public IEnumerable<FooDataItem> GetDataItems(IEnumerable<BarDto> dtos, Func<BarDto, object> key, Func<BarDto, object> data) 
{ 
    double totalData = dtos.Where(d => key(d) != null).Sum(data); 
    return dtos.Where(d => key(d) != null) 
      .GroupBy(key) 
      .OrderBy(d => d.Sum(data)) 
      .Select(
       o => new FooDataItem() 
       { 
       Key = o.Key, 
       Total = o.Sum(data), 
       Percentage = o.sum(data)/totalData 
       }); 
} 

(escrito sin compilador, etc.).

Personalmente, no lo refactorizaría, ya que esto haría que el código fuera menos legible y comprensible.

2

Debería cambiar de la expresión de consulta y convertir todas sus ubicaciones, agrupar por, ordenar por y seleccionar cláusulas en lambdas. A continuación, podría crear una función que acepte cada uno de ellos como parámetros. Aquí está un ejemplo:

private static IEnumerable<FooDataItem> GetData<T>(IEnumerable<Foo> foos, Func<Foo, bool> where, Func<Foo, T> groupby, Func<IGrouping<T, Foo>, T> orderby, Func<IGrouping<T, Foo>, FooDataItem> select) 
{ 
    var query = foos.Where(where).GroupBy(groupby).OrderBy(orderby).Select(select); 
    return query; 
} 

Basándose en este código

class Foo 
{ 
    public int Id { get; set; } 
    public int Bar { get; set; } 
} 

...

List<Foo> foos = new List<Foo>(); // populate somewhere 

Func<Foo, bool> where = f => f.Id > 0; 
Func<Foo, int> groupby = f => f.Id; 
Func<IGrouping<int, Foo>, int> orderby = g => g.Sum(f => f.Bar); 
Func<IGrouping<int, Foo>, FooDataItem> select = g => new FooDataItem { Key = g.Key, BarTotal = g.Sum(f => f.Bar) }; 

var query = GetData(foos, where, groupby, orderby, select); 
1

Aquí es un método de extensión que los factores de las porciones similares de cada consulta:

public static IEnumerable<TDataItem> GetDataItems<TData, TDataItem>(
    this IEnumerable<BarDto> dtos, 
    Func<BarDto, TData> dataSelector, 
    Func<BarDto, double> numberSelector, 
    Func<TData, double, double, TDataItem> createDataItem) 
    where TData : class 
{ 
    var eligibleDtos = dtos.Where(dto => dataSelector(dto) != null); 

    var totalNumber = eligibleDtos.Sum(numberSelector); 

    return 
     from dto in eligibleDtos 
     group dto by dataSelector(dto) into dtoGroup 
     let groupNumber = dtoGroup.Sum(numberSelector) 
     orderby groupNumber descending 
     select createDataItem(dtoGroup.Key, groupNumber, groupNumber/totalNumber); 
} 

Lo usarías l Ike esto:

var itemsByName = dtos.GetDataItems(
    dto => dto.Name, 
    dto => dto.Data, 
    (name, groupTotal, groupPercentage) => new FooDataItem 
    { 
     Name = name, 
     NumberTotal = groupTotal, 
     NumberPercentage = groupPercentage 
    }); 

var itemsByColor = dtos.GetDataItems(
    dto => dto.Color, 
    dto => dto.Number, 
    (color, groupTotal, groupPercentage) => new FooDataItem 
    { 
     Color = color, 
     DataTotal = groupTotal, 
     DataPercentage = groupPercentage 
    }); 
Cuestiones relacionadas