Home > Software design >  Optimize c# foreach complexity
Optimize c# foreach complexity

Time:08-31

Alright so I`m doing a particular case here and I need to pass through a treeview with many foreaches.

After realising my code is not working properly in memory wise I decided to rewrite the code to LINQ. I have managed to get rid of the first foreaches but at the end, I cannot get rid of the signalUI foreach because it needs the Typeof(SignalUI) properties. These being said I don`t know how to do the LINQ for 2 lists at the same time. I have tried working with JOINS but had no results.

 private void signalUITextBox_textchanged(object sender, EventArgs e)
        {
            for (int j = 0; j < _numberOfSelectedSlaves; j  )
                if (_currentNode.Parent.Parent.Text == mas.Slaves[j].Name)
                {
                    int counter = 0;
                    foreach (FrameUI frame in mas.Slaves[j].FrameList)
                    {
                        int signalcounter = 0;
                        if(_currentNode.Text == mas.Slaves[j].FrameList[counter].FrameName)
                        foreach (SignalUI signalui in frame.SignalList)
                        {
                            foreach (Control i in this.Controls)
                            {
                                if (i is TextBox)
                                {
                                    var txtbox = (TextBox)sender;
                                    if (txtbox.Name == i.Name)
                                    {
                                        foreach (var k in typeof(SignalUI).GetProperties())
                                            if (signalui.SignalName   k.Name == txtbox.Name)
                                            {
                                                mas.Slaves[j].FrameList[counter].SignalList[signalcounter].SetProperty(k.Name, Convert.ToString(txtbox.Text));
                                            }
                                    }
                                }
                                    
                            }
                                signalcounter  ;
                        }
                        counter  ;
                    }
                }
        }

        private void signalUCheckBox_textchanged(object sender, EventArgs e)
    {
        var SlaveQuery = mas.Slaves.Where(slave => slave.Name == _currentNode.Parent.Parent.Text).First();
        var FrameQuery = SlaveQuery.FrameList.Where(frame => frame.FrameName == _currentNode.Text).First();
        var chkbox = (CheckBox)sender;
        var ControlQuery = this.Controls.OfType<CheckBox>().Where(control => control.Name == chkbox.Name).First();


    }

EDIT: Thanks for help. Following below answers i managed to remove all the nested loops

        private void signalUITextBox_textchanged(object sender, EventArgs e)
    {
        int j;
        int signalcounter = 0;
        int counter = 0;
        var txtbox = (TextBox)sender;
        var properties = typeof(SignalUI).GetProperties();
        var SlaveQuery = mas.Slaves.Where(slave => slave.Name == _currentNode.Parent.Parent.Text).First();
        var FrameQuery = SlaveQuery.FrameList.Where(frame => frame.FrameName == _currentNode.Text).First();
        for (j = 0; j < _numberOfSelectedSlaves; j  )
            if (_currentNode.Parent.Parent.Text == mas.Slaves[j].Name)
                break;
        foreach (FrameUI frame in mas.Slaves[j].FrameList)
        {
            if (_currentNode.Text == mas.Slaves[j].FrameList[counter].FrameName)
                break;
            counter  ;
        }
        foreach (SignalUI signalui in FrameQuery.SignalList)
        {
            var prop = properties.FirstOrDefault(p => signalui.SignalName   p.Name == txtbox.Name);
            if (prop != null)
            {
                mas.Slaves[j].FrameList[counter].SignalList[signalcounter].SetProperty(prop.Name, Convert.ToString(txtbox.Text));
            }
            signalcounter  ;
        }
        

    }

CodePudding user response:

As a general rule: Pull repeated calculations that always yield the same result within each loop iteration out of the body of the loop and to them once. The structure of your code will simplify, declutter, and you effectively reduce its inherent complexity.

The outline of your code will look like this:

private void signalUITextBox_textchanged(object sender, EventArgs e)
{
    // build a list of textboxes, but…
    // in fact, you don't need this at all if you care only of 'sender' as a TextBox
    var textBoxes = this.Controls.OfType<TextBox>().ToList();
    
    // hence:
    if (!(sender is TextBox)) return;
    var txtbox = (textBoxe) sender;

    // build a list of properties (hint: reduce the list further, if you can)
    var properties = typeof(SignalUI).GetProperties();
    // in addition, a dictionary may come handy: .ToDictionary(p => p.Name);

    // the rest of the code seems cluttered and unnecessarily complex, too,
    // but hard to refactor without detailed knowledge of your data structures
    for (int j = 0; j < _numberOfSelectedSlaves; j  )
    {
        if (_currentNode.Parent.Parent.Text == mas.Slaves[j].Name)
        {
            int counter = 0;
            foreach (FrameUI frame in mas.Slaves[j].FrameList)
            {
                int signalcounter = 0;
                if (_currentNode.Text == mas.Slaves[j].FrameList[counter].FrameName)
                    foreach (SignalUI signalui in frame.SignalList)
                    {
                        // find the matching property
                        var prop = properties.FirstOrDefault(p => signalui.SignalName   p.Name == txtbox.Name);
                        if (prop != null)
                        {
                            mas.Slaves[j].FrameList[counter].SignalList[signalcounter].SetProperty(prop.Name, Convert.ToString(txtbox.Text));
                        }
                        signalcounter  ;
                    }
                counter  ;
            }
        }
    }
}

Three nested loops less.

  • Related