-4

Is it possible to write the following code with less foreach loops?

var pensInShed = _db.Pens.Where(w => w.ShedId == selectedShedGuid).Select(s => s.PensGuid);

                foreach(var penId in pensInShed)
                {
                    var pensInWeanedShed = _db.DailyConsumptionPens.Where(w => w.PenId == penId && w.Timestamp == ConvertedDate).Select(s => s.ConsumptionPenGuid);

                    foreach(var consumptionGuid in pensInWeanedShed)
                    {
                        var matchedUnits = _db.ConsumptionUnits.Where(w => w.DailyConsumptionId == consumptionGuid);

                        foreach(var unit in matchedUnits)
                        {
                            var unitLine = unit.UnitNETWeight * unit.UnitsUsed;

                            shedTotal += unitLine;
                        }
                    }

                    var pensToUpdate = _db.DailyConsumptionPens.Where(w => w.PenId == penId && w.Timestamp == ConvertedDate).ToList();

                    foreach(var pen in pensToUpdate)
                    {
                        pen.TotalShedUsage = shedTotal;
                        _db.SaveChanges();
                    }
                }

I do not know if this is possible but I am wanting to have the same functionality with less foreach loops in there.

GeorgeB
  • 808
  • 1
  • 8
  • 26
  • 5
    This platform is not supposed to solve such problems. If you have any problem in understanding some concepts or a particular line of code please come up with a more precise question. – ckruczek Apr 04 '18 at 08:56
  • 8
    This question might be better suited for https://codereview.stackexchange.com/ – Peter B Apr 04 '18 at 08:57
  • 6
    this question is not so out of context, salty people needs to chill with the negative voting – MrVoid Apr 04 '18 at 09:02
  • @MrVoid upvote, for salty – TheGeneral Apr 04 '18 at 09:19
  • Yep, they are salty. They discouraged the people who wants to ask. – Red Wei Apr 04 '18 at 10:23
  • @MrVoid - yes, people are so quick to dismiss questions around here for what they perceive as any relatively minor deviation from the official question format. I've previously found that a question that I could answer clearly and concisely in a single sentence was closed before I could do so... I swear there are people whose SOLE 'contribution' on SO is to 'weed out' questions rather than actually *helping* anyone... – GPW Apr 05 '18 at 13:28
  • @GPW Totally right, most of SO is just farming reputation – MrVoid Apr 05 '18 at 13:33

1 Answers1

3

It's a bit hard to see what you want without some more data about the problem...

but I suspect you could do something like this (completely untested)...

But in reality, I wouldn't worry about it unless you have some sort of performance problem. The code you have is far easier to debug than this - and I doubt the performance would be too different if at all anyway. Depending on what _db is, it's possible this would result in performing the summary operation on the database which may give better performance (due to translating the LINQ to SQL), but that depends on a lot of other stuff...

foreach loops are not inherently bad. There is no reason to not use them. We all use Linq for stylistic and code brevity reasons, but in the end, if the system is working on data in memory, the compiler will end up optimizing all this looping/summing stuff in more or less the same way. 

var pensInShed = _db.Pens.Where(w => w.ShedId == selectedShedGuid).Select(s => s.PensGuid);

foreach(var penId in pensInShed)
{

    var shedTotal =
        _db.DailyConsumptionPens
            .Where(w => w.PenId == penId && w.Timestamp == ConvertedDate)
        .Sum(x =>
            _db.ConsumptionUnits
                .Where(w =>
                    w.DailyConsumptionId == x.ConsumptionPenGuid)
                .Sum(xx => xx.UnitNETWeight * xx.UnitsUsed)
        );


    var pensToUpdate = _db.DailyConsumptionPens
        .Where(w => w.PenId == penId && w.Timestamp == ConvertedDate)
        .ToList();

    foreach(var pen in pensToUpdate)
    {
        pen.TotalShedUsage = shedTotal;
        _db.SaveChanges();
    }
}
GPW
  • 2,528
  • 1
  • 10
  • 22
  • Thanks for the information, I just thought that using this many foreach loops is a bad idea, but the performance is not affected, so I guess I should just use comments in the code if I want to make it more understandable in the future. – GeorgeB Apr 04 '18 at 10:22
  • 1
    also moving some logic into methods and use descriptive names may help you more than comment the code – MrVoid Apr 04 '18 at 10:28
  • 1
    @GeorgeB no problem. It's too easy to get hung up on wanting to do everything *perfectly*... then you end up never getting anything done. The bottom line is that you could spend 10x as long and end up with something that doesn't actually execute any faster - just 'looks' a bit nicer. Even worse, when you (or someone else) later come back to modify it or fix a bug, it'll take far longer to work out what it's doing.... – GPW Apr 05 '18 at 13:24