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

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
{
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
{
cmbNetwork.Focus();
}
if(TestVal(cmbRegion.SelectedIndex.ToString()))
{
inVars.region = cmbRegion.SelectedIndex.ToString();
}
else
{
cmbRegion.Focus();
}
if(TestVal(cmbSupplier.SelectedIndex.ToString()))
{
inVars.supplier = cmbRegion.SelectedIndex.ToString();
}
else
{
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;
}

}


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

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();
}
}

• I'll give it a shot but I'm pretty noobish – Dani Aug 15 '11 at 13:18
• @Dani Just use the properties window in visual studio, select the control and scroll through properties to find the Tag property which you will put your error text in, and the SelectedIndexChanged event which you will put "OnSelectedIndexChanged" or whatever you name your event handler into and same for the text box, you just put the name of the method which has this signature public void MethodName(object sender, EventArgs e) into the event in the Properties window. – Jimmy Hoffa Aug 15 '11 at 14:32
• I'll definitely try it out – Dani Aug 16 '11 at 10:15
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;
}
}


• I kinda dislike the fact that a method named IsEmpty` has to wait for a message box to be dismissed before returning the result. – Sorin Comanescu Aug 19 '11 at 10:38