Click here to Skip to main content
15,867,568 members
Please Sign up or sign in to vote.
1.00/5 (1 vote)
Hi,
I have a concern about declaring variable(s) inside a for/foreach/while/dowhile loop which would last for more than 1000~10000 iterations.

We won’t bother about declaring any number of variables (string[],List<t>, Dictionary<key,value>) (forgot about creating a custom type and adding to a list) and keep on instantiating it for each and every time when the loops iterates. And we don’t have (know either) any tool to point out if this was correct or wrong.

My question here is, anyone have any concern about this? Are we following the correct way of using something like this?

The reason for this post. Because I do this and I want to know whether I was following a correct way of coding.

Please let me know if any one can't understand this question.

C#
foreach (var cItem in DataQualityMonitoringViewDataGrid.VisibleColumns.Where(lwhr => lwhr.SortDirection != SortDirection.None))
                {
                    //Inner scope variable
                    var statusProcessing = string.Format("Processing {0} Row", row);

                    //Inner scope variable
                    var fName = cItem.FieldName.Replace(".ValueText", "").ToUpper();
                    if (fName == Constants.DG_COL_STATUS) continue;
                    fName = cItem.FieldName.Replace(".ValueText", "");

                    //Inner scope variable
                    string xmlDbApiFName;
                    CommonUtils.ColumnCollection.TryGetValue(cItem.FieldName.ToUpper(), out xmlDbApiFName);

                    if (string.IsNullOrEmpty(xmlDbApiFName)) break;

                    //Inner scope variable
                    var currstatusDQmCell = currentCol.Columns(fName) as DQMCell;
                    //Inner scope variable
                    var currStatusVal = currstatusDQmCell.Columns("ValueText");

                    if (string.IsNullOrEmpty(currStatusVal.ToString()))
                        condition += @" not(" + xmlDbApiFName + " != '') and ";
                    else
                        condition += @" contains(" + xmlDbApiFName + ",'" + currStatusVal + "') and ";

                    //Inner scope variable
                    const string conditionRed = " contains(_x0031_5010,'0')";

                    //Inner scope variable
                    var forRed = CommonUtils.XmlDocument.SelectNodes("//Distinct_DQM[" + condition + conditionRed + "]");

                    if (forRed != null && forRed.Count >= 1)
                        ApplyCellColor(currentCol, CommonUtils.GetColor("0"), fName.ToUpper(), DQMFlag.red);    // Local Member
                    else
                    {
                        //Inner scope variable
                        const string conditionAmber = " contains(_x0031_5010,'1')";

                        //Inner scope variable
                        var foramberd = CommonUtils.XmlDocument.SelectNodes("//Distinct_DQM[" + condition + conditionAmber + "]");  //Static member
                        if (foramberd != null && foramberd.Count >= 1)

                            ApplyCellColor(currentCol, CommonUtils.GetColor("1"), fName.ToUpper(), DQMFlag.amber);      // Local Member
                        else
                        {
                            //Inner scope variable
                            var dQmFlag = DQMFlag.amber;
                            Enum.TryParse(currentCol.Status_15010.ValueText, out dQmFlag);
                            ApplyCellColor(currentCol, currentCol.Status_15010.Background, fName.ToUpper(), dQmFlag);       // Local Member
                        }
                    }
                    column++;
                    currentCol.IsHeatMapApplied = true;
                }
Posted
Updated 3-Mar-15 20:31pm
v3
Comments
Kunal Chowdhury «IN» 4-Mar-15 2:21am    
It depends. Share us a piece of code, so that we can check
Gold$Coin 4-Mar-15 2:30am    
No, I just have concern that if this way will be good or bad. I just came up with this taught and would like to conclude on this after making sure that am rite. And any way i have pasted a sample code that i came accross which was consuming more memory and time.
Gold$Coin 4-Mar-15 22:25pm    
Hope everthing was good in my god? please let me know. you are the expert and let me learn something from you.

1 solution

There is nothing wrong with declaring many variables inside loop, as the optimizer can easily take care of it. If all those variables are reasonable, you should not to "manual optimization", such as carrying them out of the loop. Just the opposite, it's good to limit format context of all variable to the fragment of code where they are really needed.

At the same time, your many variables can be a result of poorly algorithmic mistakes, and that could be bad. Without your code sample, it's hard to add more.

—SA
 
Share this answer
 
Comments
Gold$Coin 4-Mar-15 22:24pm    
have pasted the code please have a look.
Sergey Alexandrovich Kryukov 4-Mar-15 23:48pm    
It's hard to estimate all the detail from the first glance, its correctness, and things like that, but in the aspect you raised, the concern of the stack variables created inside the loop, I see no problems at all, all your "var" look reasonable.
—SA
Gold$Coin 5-Mar-15 5:09am    
Thanks Sergey.
Sergey Alexandrovich Kryukov 5-Mar-15 8:36am    
You are very welcome.
—SA

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900