Group collections by their elements property

Question

I have a Customer class:

public class Customer
{
    public Guid Id { get; set; }
    // Some other properties...}

And three transactions classes that have a reference to Customer:

public class Order
{
    public Guid CustomerId { get; set; }
    // Some other properties...}

public class Invoice
{
    public Guid CustomerId { get; set; }
    // Some other properties...}

public class Payment
{
    public Guid CustomerId { get; set; }
    // Some other properties...}

I have a collection for each of transaction type. And I want to get this collection to be "grouped" by their elements CustomerId properties. So, as a result I'd like to get a collection of such objects:

public class CustomerInfo
{
    public CustomerInfo(Guid customerId, IEnumerable<Order> orders,
        IEnumerable<Invoice> invoices, IEnumerable<Payment> payments){...}

    public Guid CustomerId { get; set; }
    public IEnumerable<Invoice> Invoices { get; set; }
    public IEnumerable<Order> Orders { get; set; }
    public IEnumerable<Payment> Payments { get; set; }
}

Right now I am doing so by this function:

private IEnumerable<CustomerInfo> _GetCustomerInfo(
    IEnumerable<Payment> payments, IEnumerable<Invoice> invoices,
    IEnumerable<Order> orders)
{
    var invoicesGroupdByCustomers = invoices.GroupBy(x => x.CustomerId);
    var ordersGroupdByCustomers = orders.GroupBy(x => x.CustomerId);
    var paymentsGroupdByCustomers = payments.GroupBy(x => x.CustomerId);

    var result = new List<CustomerInfo>();
    foreach (var group in invoicesGroupdByCustomers)
        result.Add(new CustomerInfo(group.Key, 
            ordersGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key), 
            group,
            paymentsGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key)));

    foreach (var group in ordersGroupdByCustomers)
        if (!result.Any(x => x.CustomerId == group.Key))
            result.Add(new CustomerInfo(group.Key, 
                group,
                invoicesGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key),
                paymentsGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key)));

    foreach (var group in paymentsGroupdByCustomers)
        if (!result.Any(x => x.CustomerId == group.Key))
            result.Add(new CustomerInfo(group.Key,
                ordersGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key),
                invoicesGroupdByCustomers.FirstOrDefault(x => x.Key == group.Key),
                group));

    return result;
}

Here is sample of calculations where I use these transactions collections:

private Dictionary<Guid, decimal> _CalculateSales(
    IEnumerable<CustomerInfo> customersSalesInfo)
{
    var result = new Dictionary<Guid, decimal>();

    foreach (var customerInfo in customersSalesInfo)
    {
        var sales = customerInfo.Invoices.Sum(x => x.Header.SubTotalAmt) +
            customerInfo.Orders.Sum(x => x.Header.SubTotalAmt) -
            customerInfo.Payments.Sum(x => x.Header.SubTotalAmt);
        result.Add(customerInfo.CustomerId, sales);
    }

    return result;
}

Is there a better approach?

Answer 1

If you're developing a production solution I would rather think how to avoid loading all the customers/invoices/etc at first place. If that's the case then please describe the use cases in which Customerinfo is going to be used, how do you get invoices/orders/payments collections, and is there a chance to leverage ORM capabilities (assuming that you get the data via some sort of ORM) to load these collections for customers?

But if we are just talking about programming exercise then I would probably go with the following solution (influenced by RavenDB's map/reduce approach):

IEnumerable<CustomerInfo> infos = (
    from invoice in invoices
    select new { invoice.CustomerId, Invoice = invoice, Order = null, Payment = null }
    ).Concat(
        from order in orders
        select new { order.CustomerId, Invoice = null, Order = order, Payment = null }
    ).Concat(
        from payment in payments
        select new { payment.CustomerId, Invoice = null, Order = null, Payment = payment }
    ).GroupBy(x=>x.CustomerId, (key, group) => new CustomerInfo(key, 
        group.Select(x => x.Invoice).Where(i => i != null),
        group.Select(x => x.Order).Where(o => o != null),
        group.Select(x => x.Payment).Where(p => p != null));

I can't test this code right now, you might have to explicitly specify types in anonymous objects.

Update: Based on your usage of CustomerInfo you might not actually need it at all (if that's all you do with it) :). I would rather try to reduce the amount of data straight away, thus improving performance and reducing memory usage:

private Dictionary<Guid, decimal> _CalculateSales(
    IEnumerable<Payment> payments, IEnumerable<Invoice> invoices,
    IEnumerable<Order> orders)
{
    var result = (
        from invoice in invoices
        select new { invoice.CustomerId, Amount = invoice.Header.SubTotalAmt}
        ).Concat(
            from order in orders
            select new { order.CustomerId, Amount = order.Header.SubTotalAmt}
        ).Concat(
            from payment in payments
            select new { payment.CustomerId, Amount = -payment.Header.SubTotalAmt}
        ).GroupBy(x => x.CustomerId)
        .ToDictionary(g => g.Key, g => g.Sum(x => x.Amount));
    return result;
}

The benefit of this solution comparing to original code is that it iterates over collections only once, and the only Dictionary created is the actual result.

Answer 2

If they all implemented a common interface then you could simplify it.

 public interface ICustomerItem
    {
        Guid CustomerId {get;}
     }

Concat the the items together

var items = invoices.Concat(payments).Concat(orders);

Then project them into your class

 items
.GroupBy(x => x.CustomerId)
.Select(g => 
          new CustomerInfo(g.Key, 
                 g.OfType<Order>(), 
                 g.OfType<Payment>(),
                 g.OfType<Invoice>())

Answer 3

I think a better way would be to extract all of the ids first and then create your result based on that:

private IEnumerable<CustomerInfo> GetCustomerInfo(
    IEnumerable<Payment> payments, IEnumerable<Invoice> invoices,
    IEnumerable<Order> orders)
{
    // if the parameters were ILists, this wouldn't be necessary
    payments = payments.ToArray();
    invoices = invoices.ToArray();
    orders = orders.ToArray();

    var customerIds = payments.Select(p => p.CustomerId)
        .Concat(invoices.Select(i => i.CustomerId))
        .Concat(orders.Select(o => o.CustomerId))
        .Distinct()
        .ToArray();

    // using ToLookup() means we can efficiently retrieve items based on id
    var paymentsLookup = payments.ToLookup(p => p.CustomerId);
    var invoicesLookup = invoices.ToLookup(i => i.CustomerId);
    var ordersLookup = orders.ToLookup(o => o.CustomerId);

    return customerIds.Select(
        id => new CustomerInfo(id, ordersLookup[id], invoicesLookup[id], paymentsLookup[id]));
}

Answer 4

Maybe I'm missing something but it seems that your code have strange behavior. If orders and payments collections have items with same CustomerId as in invoices collection then by checking

if (!result.Any(x => x.CustomerId == group.Key))

you will lose all of them and result collection will contain one CustomerInfo element per customer with some invoices and orders == null, payments == null.

If this behavior is ok I would recommend using Dictionary to avoid multiple list scanning with Any()

private IEnumerable<CustomerInfo> _GetCustomerInfo(
        IEnumerable<Payment> payments,
        IEnumerable<Invoice> invoices,
        IEnumerable<Order> orders)
    {
        if (payments == null) throw new ArgumentNullException("payments");
        if (invoices == null) throw new ArgumentNullException("invoices");
        if (orders == null) throw new ArgumentNullException("orders");

        var invoicesGroupdByCustomers = invoices.GroupBy(x => x.CustomerId);
        var ordersGroupdByCustomers = orders.GroupBy(x => x.CustomerId);
        var paymentsGroupdByCustomers = payments.GroupBy(x => x.CustomerId);

        var result = new Dictionary<Guid, CustomerInfo>();
        foreach (var group in invoicesGroupdByCustomers)
            result.Add(group.Key, new CustomerInfo(group.Key, null, group, null));

        foreach (var group in ordersGroupdByCustomers)
            if (!result.ContainsKey(group.Key))
                result.Add(group.Key, new CustomerInfo(group.Key, group, null, null));

        foreach (var group in paymentsGroupdByCustomers)
            if (!result.ContainsKey(group.Key))
                result.Add(group.Key, new CustomerInfo(group.Key, null, null, group));

        return result.Values;
    }

Hope this helps

Answer 5

You could take your original approach and use .ToList()

var invoicesGroupdByCustomers = invoices.GroupBy(x => x.CustomerId).ToList();
var ordersGroupdByCustomers = orders.GroupBy(x => x.CustomerId).ToList();
var paymentsGroupdByCustomers = payments.GroupBy(x => x.CustomerId).ToList();

Just another option to be aware of. I hope this helps.