CODEWITHSUNDEEP
c#linqforeachiterationoverwrite
shubniggurath
shubniggurath

Reputation: 966

foreach to update values in object overwrites values

Caveat: I know some parts of this code are bad, but it is what it is for now. I just want it to run properly. I fully intend to refactor later. I just need a working app right now.

The initial linq query grabs several fields of data, but more data must be added per item in the resultset. So, we have the foreach below. It grabs data and updates each row.

It overwrites everything to what I'm thinking is probably the last iteration of the foreach. Why? How do I keep it from overwriting?

Keep in mind that the working variable just meabns a period id. I want to get previous or future periods, and subtracting or adding to this allows this. 

    public List<intranetGS.Forecast> getForecast(int branchId) {
        //user role protection here

        intraDataContext q = new intraDataContext();

        //this grabs the initial data
        var basefc = (from f in q.fc_items
                      where f.color_option == false
                      select new intranetGS.Forecast {
                          itemId = f.item_id,
                          item = f.item_number,
                          itemDesc = f.description,
                          itemSuffix = f.item_suffix,
                          itemPrefix = f.item_prefix,
                          designation = f.designation
                      });


        //now we filter
        switch (getDesignation(branchId)) {
            case 1:
                basefc = basefc.Where(n => n.designation != 3);
                basefc = basefc.Where(n => n.designation != 6);
                break;
            case 2:
                basefc = basefc.Where(n => n.designation > 3);
                break;
            case 3:
                basefc = basefc.Where(n => n.designation != 2);
                basefc = basefc.Where(n => n.designation != 6);
                break;
        }

        var current = Convert.ToInt32(DateTime.Now.Month);
        var working = 0;
        var year = Convert.ToInt32(DateTime.Now.Year);

        List<intranetGS.Forecast> res = new List<intranetGS.Forecast>(); 

        foreach (var f in basefc) {

            working = getPeriod(current + "/" + (year - 1)); //starting with last year;
            var ly = (from l in q.fc_forecasts where l.period == working && l.branch == branchId && l.item == f.itemId select l).FirstOrDefault();
            if (!object.ReferenceEquals(ly, null)) {
                f.lastYearForecast = ly.forecast;
                f.lastYearReceipt = ly.receipt;
            }

            working = getPeriod(current + "/" + year) - 2; //two months ago
            var m2 = (from l in q.fc_forecasts where l.period == working && l.branch == branchId && l.item == f.itemId select l).FirstOrDefault();
            if (!object.ReferenceEquals(m2, null)) {
                f.twoMosForecast = m2.forecast;
                f.twoMosReceipts = m2.receipt;
                f.twoMosUsage = m2.usage_lb;
            }

            working = getPeriod(current + "/" + year) - 1; //one month ago
            var m1 = (from l in q.fc_forecasts where l.period == working && l.branch == branchId && l.item == f.itemId select l).FirstOrDefault();
            if (!object.ReferenceEquals(m1, null)) {
                f.oneMosForecast = m1.forecast;
                f.oneMosReceipts = m1.receipt;
                f.oneMosUsage = m1.usage_lb;
            }

            working = getPeriod(current + "/" + year); //current month
            var m = (from l in q.fc_forecasts where l.period == working && l.branch == branchId && l.item == f.itemId select l).FirstOrDefault();
            if (!object.ReferenceEquals(m, null)) {
                f.currentMosForecast = m.forecast;
                f.currentMosReceipts = m.receipt;
                f.currentMosusage = m.usage_lb;
            }

            working = getPeriod(current + "/" + year) + 1; //one month from now
            var mnext1 = (from l in q.fc_forecasts where l.period == working && l.branch == branchId && l.item == f.itemId select l).FirstOrDefault();
            if (!object.ReferenceEquals(mnext1, null)) {
                f.plusOneForecast = mnext1.forecast;
                f.plusOneForecastId = mnext1.forcast_id;
            }

            working = getPeriod(current + "/" + year) + 2; //two months from now
            var mnext2 = (from l in q.fc_forecasts where l.period == working && l.branch == branchId && l.item == f.itemId select l).FirstOrDefault();
            if (!object.ReferenceEquals(mnext2, null)) {
                f.plusTwoForecast = mnext2.forecast;
                f.plusTwoForecastId = mnext2.forcast_id;
            }

        } //this is insanely and extremely cumbersome; refactor later.

        return basefc;
    }

UPDATE: It wasn't a list, it needed to be a list to avoid the overwrite.

Upvotes: 0

Views: 450

Answers (1)

ΩmegaMan
ΩmegaMan

Reputation: 31596

The issue is that there is a delayed execution which occurs in linq as the user is building the query and internally it is building an expression tree where new expressions can be added. Once the query factors are settled upon during an execution, such as an enumerable target in a for loop or via .ToList() that list is fluid still. Since the code was simply adding more expressions and not filtering it out into a new list, the query just grew.

The question is when working on existing code, did the developer want to keep building the expression tree for performance or did they intend to make the items concrete at each step along the process?.

You may be fixing an issue by making the initial list concrete but could be introducing a logic bug going forward. Keep that in mind.

Upvotes: 1

Related Questions