Do I use a break to escape the loop or is there a better way to solve this problem?-CodePudding

if (e.KeyCode == Keys.Enter)
{
    string strCurrentString = textBoxInput.Text.Trim().ToString();
    if (strCurrentString != "")
    {
        if (radioButtonPick.Checked == true && radioButtonNew.Checked == true)
        {
            foreach (DataGridViewRow row in excelviewgrid.Rows)
            {
                if (strCurrentString == row.Cells["Material"].Value.ToString())
                {
                    int value = Convert.ToInt32(comboBoxmultiply.Text);
                    for (int i = 0; i < value; i  )
                    {
                        listBoxinput.Items.Add(textBoxInput.Text   " | "   row.Cells["Material Description"].Value   " (-1 New Pick)");
                    }
                }
                else 
                {
                    MessageBox.Show("Wrong Article");
                }
            }
        }
        comboBoxmultiply.SelectedIndex = 0;
        textBoxInput.Clear();
    }
}

At the moment the MessageBox from the else statement always triggers no matter whether the if statement is true or false and is stuck on the screen. I presume because of the foreach loop. How do I solve this in a good way? Do I just break; after the else statement? Multiple breaks? Or different code?

CodePudding user response：

break is the simplest solution, but we need a flag or some other variable to determine that the loop completed due to the break, and not due to the loop completing normally:

if (e.KeyCode == Keys.Enter)
{
    string strCurrentString = textBoxInput.Text.Trim().ToString();
    if (strCurrentString != "")
    {
        if (radioButtonPick.Checked == true && radioButtonNew.Checked == true)
        {
            bool found = false;
            foreach (DataGridViewRow row in excelviewgrid.Rows)
            {
                if (strCurrentString == row.Cells["Material"].Value.ToString())
                {
                    found = true;
                    int value = Convert.ToInt32(comboBoxmultiply.Text);
                    for (int i = 0; i < value; i  )
                    {
                        listBoxinput.Items.Add(textBoxInput.Text   " | "   row.Cells["Material Description"].Value   " (-1 New Pick)");
                    }
                    break;
                }
            }

            if (!found)
                MessageBox.Show("Wrong Article");
        }
        comboBoxmultiply.SelectedIndex = 0;
        textBoxInput.Clear();
    }
}

CodePudding user response：

You should not loop all grid-rows and show a MessageBox everytime that it was not the material you searched. Instead first loop all rows and show the message-box afterwards. And yes, use break to exit the loop if you have found the row.

You could use code like this:

if (e.KeyCode == Keys.Enter)
{
    AddMaterialsToListBox(textBoxInput.Text.Trim().ToString(), Convert.ToInt32(comboBoxmultiply.Text));
    comboBoxmultiply.SelectedIndex = 0;
    textBoxInput.Clear();
}

// .....

void AddMaterialsToListBox(string materialName, int count)
{
    DataGridViewRow materialRow = null;
    foreach (DataGridViewRow row in excelviewgrid.Rows)
    {
        if (materialName == row.Cells["Material"].Value.ToString())
        {
            materialRow = row;
            break;
        }
    }
    
    if(materialRow == null)
    {
        MessageBox.Show("Wrong Article");
        return;
    }
    
    for (int i = 0; i < count; i  )
    {
        listBoxinput.Items.Add(materialName   " | "   materialRow.Cells["Material Description"].Value   " (-1 New Pick)");
    }
}