2010-06-03 6 views
8

J'essaie de comprendre comment refactoriser correctement ce code LINQ. Ce code et d'autres codes similaires se répètent dans le même fichier ainsi que dans d'autres fichiers. Parfois, les données manipulées sont identiques et parfois les données changent et la logique reste la même.Comment refactoriser ce code LINQ dupliqué?

Voici un exemple de logique dupliquée fonctionnant sur différents champs d'objets différents.

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 
      }; 
} 

Quelqu'un at-il un bon moyen de refactoriser cela?

+0

Est-ce que les noms de propriété dans 'FooDataItem' doivent être différentes? La solution serait plus simple si elles étaient plus génériques, par ex. 'Key',' Total', 'Pourcentage'. –

+0

+1 Si c'est une représentation exacte, alors vos méthodes sont petites et font exactement ce que vous attendez. Peut-être que d'autres parties de votre code pourraient bénéficier d'une refactorisation en premier? –

Répondre

10

Quelque chose comme ceci:

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)); 
} 

Dans votre exemple, vous pouvez l'appeler comme ceci:

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 vous avez changé FooDataItem être plus générique, il serait plus facile.

+1

c'est beau code. – Femaref

+1

Nice. Pour plus de lisibilité et pour éviter d'avoir à refactoriser tous les appels de fonction existants, j'appliquerais probablement votre appel à 'GetDataItems' dans une fonction' GetDataItemsByColor (IEnumerable dtos) '. – Jelly

1

Je pense que si vous avez refactorisé cela, il serait plus difficile à lire que ce que vous avez déjà. Tout ce que je peux penser à l'un ou l'autre implique Linq dynamique, ou modifier ou encapsuler BarDto pour avoir une sorte d'élément spécialisé à utiliser pour rien, mais le regroupement.

3

Je n'utiliserais pas la syntaxe de requête pour cela, utilisez la chaîne de méthode.

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 
       }); 
} 

(écrit sans compilateur, etc.).

Personnellement, je ne le retravaillerais pas, car cela rendrait le code moins lisible et compréhensible.

2

Vous devez passer de l'expression de requête et convertir toutes vos clauses where, group by, order by et select en lambdas. Vous pouvez ensuite créer une fonction qui accepte chacun d'eux en tant que paramètres. Voici un exemple:

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; 
} 

Sur la base de ce code

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

Voici une méthode d'extension qui factorise les parties similaires de chaque requête:

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); 
} 

Vous l'utiliseriez l ike ceci:

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 
    }); 
Questions connexes