How to store different GroupBy in one variable with minimal code duplication?

Question

I have the following piece of code:

  var newProducts = _summaryRepository.GetFilteredSummaries(manufacturerIds, countryIds, categoryIds,
                    null, widgetIds, startDate, null).Where(s => s.Product.ProductCountries.FirstOrDefault(pc => pc.CountryId == s.CountryId).CreatedAt >= startDate.Value)
                    .GroupBy(s => new { widgetId = s.widgetId, ProductId = s.ProductId });

If I have the condition Show all I want to take the following out of the GroupBY:

WidgetId = s.WidgetId

So the it would now be:

var newProducts = _summaryRepository.GetFilteredSummaries(manufacturerIds, countryIds, categoryIds,
                    null, widgetIds, startDate, null).Where(s => s.Product.ProductCountries.FirstOrDefault(pc => pc.CountryId == s.CountryId).CreatedAt >= startDate.Value)
                    .GroupBy(s => new {ProductId = s.ProductId });

There is a lot of code which is reliant on newProducts and when I have created an if statement and put the var newProducts in the outerscrope it stops everything working.

I know this is probably a silly question but how can I do this with the minimal repetitive code?

Is it that I'm declaring the variable wrong when I do this:

var newProducts;     
 if(model.allWidgets)
    {newProducts = _summaryRepository.GetFilteredSummaries(manufacturerIds, countryIds, categoryIds,
                            null, WidgetIds, startDate, null).Where(s => s.Product.ProductCountries.FirstOrDefault(pc => pc.CountryId == s.CountryId).CreatedAt >= startDate.Value)
                            .GroupBy(s => new {ProductId = s.ProductId });}
else
{
newProducts = _summaryRepository.GetFilteredSummaries(manufacturerIds, countryIds, categoryIds,
                        null, WidgetIds, startDate, null).Where(s => s.Product.ProductCountries.FirstOrDefault(pc => pc.CountryId == s.CountryId).CreatedAt >= startDate.Value)
                        .GroupBy(s => new { WidgetId = s.WidgetId, ProductId = s.ProductId });
}

your refactoring depends much on how you use `newProducts`. As you see `newProducts` can be totally different type (branched differently). That means your code is becoming ***dynamic*** (later on). — Hopeless, Sep 23 '15 at 14:12

score 4 · Accepted Answer · answered Sep 23 '15 at 14:19

4

You can change your GroupBy to ignore s.WidgetId when model.allWidgets is true:

query.GroupBy(s => new { WidgetId = model.allWidgets ? 0 : s.WidgetId, ProductId = s.ProductId });

answered Sep 23 '15 at 14:19

Jakub Lortz

14,616
3
25
39

This seems to be exactly what I m looking for but for some reason my model.allWidgets is coming back as undefined if its not checked so just trying to figure out why that is... – anna Sep 23 '15 at 15:12
sorry something I missed in the view works perfectly thank you – anna Sep 23 '15 at 15:29

score 2 · Answer 2 · edited May 23 '17 at 10:27

From LINQ Conditional Group, you can add a "null" to the grouping (WidgetId = 0), causing the GroupBy to return the same anonymous grouping type in both cases:

var newProducts =  _summaryRepository.GetFilteredSummaries(...)
                                     .Where(s => ...)

var groupedProducts = newProducts.GroupBy(s => 
{
    if(model.allWidgets)
    {
        return new 
        {
            ProductId = s.ProductId,
            WidgetId = 0,
        };
    }
    else
    {
        return new 
        {
            ProductId = s.ProductId,
            WidgetId = s.WidgetId,
        };
    }
});

And of course, as the just-added answer indicates, this can be greatly reduced using the conditional operator:

var groupedProducts = newProducts.GroupBy(s => 
    new 
    {
        ProductId = s.ProductId,
        WidgetId = model.allWidgets ? 0 : s.WidgetId,
    });

This type is still anonymous and thus not directly accessible from your code, so in order to return it from a method, introduce a class to hold the group:

public class ProductGrouping
{
    public int ProductId { get; set; }
    public int? WidgetId { get; set; }
}

public IGrouping<ProductGrouping, Summary> GetGroupedSummaries()
{
    return _summaryRepository.GetFilteredSummaries(...)
                             .Where(s => ...)
                             .GroupBy(s => new ProductGrouping
                             {
                                ProductId = s.ProductId,
                                WidgetId = model.allWidgets ? (int?)null : s.WidgetId,
                             });
}

But since the `group by`s return different types in each case, how would you define a common `IGrouping`? — sstan, Sep 23 '15 at 14:03

sstan · Answer 3 · 2015-09-23T14:14:42.580

What you are trying to do is not really possible because your 2 queries return different anonymous types, unless you are willing to assign the results back into a variable of type object, which doesn't seem very useful (or dynamic, but then you lose the compile time benefits of using LINQ).

To be able to use the var keyword, the compiler must be able to determine the type on declaration, which means you have to provide an assignment right where you declare your variable.

As stated in the documentation for Implicitly Typed Local Variables:

The var keyword instructs the compiler to infer the type of the variable from the expression on the right side of the initialization statement.

And also...

It is important to understand that the var keyword does not mean "variant" and does not indicate that the variable is loosely typed, or late-bound. It just means that the compiler determines and assigns the most appropriate type.

ok so unfortunately I will need to repeat code there's not really a way round it is that correct? — anna, Sep 23 '15 at 14:12
Not unless you're willing to sacrifice compile time checking and early binding. — sstan, Sep 23 '15 at 14:15

How to store different GroupBy in one variable with minimal code duplication?

3 Answers3