# IEnumerable - Loop all child records (LineItems) per Invoice - example included - architect said inefficient

The goal of this question is to pick up proper coding skills from a professional and become a better developer

An architect said my code was not efficient.

The idea is to loop list of InvoiceData,
each Invoice has line items, then I must perform a calculation based on the LineItems per Invoice.

public IEnumerable<InvoiceSummary> GetInvoiceSummary()
{
List<InvoiceSummary> lis = new List<InvoiceSummary>();
foreach(Invoice t  in InvoiceData)
{
InvoiceSummary InS = new InvoiceSummary();
InS.Id = t.Id;
InS.SalesPercentage = ((double)t.Items / (double)t.Visits) * 100;
int LineItemCount = 0;
int LineItemCost = 0;
foreach (LineItem d in t.LineItems)
{
LineItemCount += d.LineItemCount;
LineItemCost += d.LineItemCost;
}
InS.LineItemAvgCost = ((double)LineItemCount / (double)LineItemCost);
}
}

How would you have done it?
Walter Ritzel

I would have used a SQL to do that, if you are not using Entity Framework. If EF is being used, I would have written a LINQ query.
jxharding

OK can you give and hints on the LINQ please, I tried to get the sum of the 2 columns from LIneItems but could not succeed, and reverted to above methods
it_saige

membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Why do you cast all your int to double?

You do not have to do that, the result of a division of 2 numbers is automatically a double, no matter the type of numeric value you have.

You are also declaring 2 new variables LineItemCount and LineItemCost each time you go through the main loop. I would rather declare these variable once, before entering the loop, and reset them to zero before entering the inner loop.
@James - You have to cast because straight integer division removes the remainder.  Proof of concept.
using System;

namespace EE_Q28699687
{
class Program
{
static void Main(string[] args)
{
Console.WriteLine(1 / 2);
Console.WriteLine((double)1 / (double)2);

}
}
}
Produces the following output -You don't *have* to cast both sides (because of widening conversion) but I usually do it just so that anyone coming behind me understands my intent.  Also casting after the division will not reinstate the remainder; e.g.
Console.WriteLine((double)(1 / 2));
Still produces an output of zero.

-saige-