Is this validation sufficient to ensure that the vals are not empty?

Question

I'm working on a small winforms app, the goal of which is to capture data and write to xml. I'm still a child where programming is concerned so could you guys please take a look and suggest changes as I'm pretty sure that this is not a good approach. I'm using a masked-textbox for the invoice number which should only contain numbers. I'm trying not to use a numupdown for obvious reasons.

InputVars is a public class with empty string properties (string value {get;set;})

    private bool TestVal(string testedVal)
    {
        if (!String.IsNullOrEmpty(testedVal))
        {
            return true;
        }
        else
        {
            return false;
        }
    }



    private InputVars SetSemVars()
    {
        InputVars inVars = new InputVars();
        if (TestVal(txtCapt.Text))
        {
            inVars.captured = txtCapt.Text;
        }
        else
        {
            MessageBox.Show("Please insert your name here");
            txtCapt.Focus();
        }
        if (TestVal(mtxtInvoiceNumber.Text))
        {
            inVars.invoice = mtxtInvoiceNumber.Text;
        }
        else
        {
            MessageBox.Show("Please insert the invoice number here");
            mtxtInvoiceNumber.Focus();
        }
        if(TestVal(cmbNetwork.SelectedIndex.ToString()))
        {
            inVars.network = cmbNetwork.SelectedIndex.ToString();
        }
        else
        {
            MessageBox.Show("Please select the network ");
            cmbNetwork.Focus();
        }
        if(TestVal(cmbRegion.SelectedIndex.ToString()))
        {
            inVars.region = cmbRegion.SelectedIndex.ToString();
        }
        else
        {
            MessageBox.Show("Please select your office code here");
            cmbRegion.Focus();
        }
        if(TestVal(cmbSupplier.SelectedIndex.ToString()))
        {
            inVars.supplier = cmbRegion.SelectedIndex.ToString();
        }
        else
        {
            MessageBox.Show("Please select the supplier here");
            cmbSupplier.Focus();
        }
        return inVars;
    }

I was playing around earlier and got this to work , just thought I'd share

        foreach (TextBox box in this.panel1.Controls.OfType<TextBox>())
        {
            if (String.IsNullOrWhiteSpace(box.Text))
            {
                box.Text = "Enter value here";
                //or do a messagebox etc.
                box.Focus();
                return;
            }

        }

that`s loads and loads shorter and easier to read and makes sure that I don't forget a value... me gusta

Answer 1

Your testval method is completely unnecessary, just do your validation right in the method. I would create an eventhandler for each type of control's onchanged event, then you'd have one for combo boxes and one for text boxes. Have each control's tag property hold the invalid message for it to be displayed. Just set all text and combo boxes you want to validate for non-null/whitespace to have the same event handler to avoid code duplication.

Something like so:

private void OnTextChanged(object sender, EventArgs e)
{
  TextBox textBoxToValidate = sender as TextBox;
  if (string.IsNullOrWhiteSpace(textBoxTovalidate.Text))
  {
    MessageBox.Show(textBoxToValidate.Tag.ToString());
    textBoxToValidate.Focus();
  }
}

private void OnSelectedIndexChanged(object sender, EventArgs e)
{
  ComboBox comboBoxToValidate = sender as ComboBox;
  if (string.IsNullOrWhiteSpace(comboBoxTovalidate.SelectedIndex.ToString()))
  {
    MessageBox.Show(comboBoxToValidate.Tag.ToString());
    comboBoxToValidate.Focus();
  }
}

Answer 2

1. Don't use String.IsNullOrEmpty to validate required values.

2. You can create a method

   private bool IsEmpty(string value, string errorMessage, Control controlToValidate)
   {
       if ((value ?? string.Empty).Trim().Length == 0)
       {
           MessageBox.Show(errorMessage);
           controlToValidate.Focus();
           return true;
       }
       else
       {
           return false;
       }
   }
   

   instead of TestVal.

3. Consider using Validating event for validating your controls and the Tag property to associate error messages with the controls.
4. Consider using ErrorProviders.