Trying to check whether value has been inputted in the text boxes

Question

I am trying to figure out what is wrong with my code here, i am trying to do data validation by checking if the user has inputted any values in the text box, but when i run the code it will carry on saving the card regardless if there are values entered or not inside the text boxes.

private void buttonSave_Click(object sender, EventArgs e)
{
    nameChosenOnCard  = textBoxName.Text;
    numOfCard = textBoxCardNum.Text;
    cardStartDate = textBoxStartMonth.Text + "/" + textBoxStartYear.Text;
    cardEndDate = textBoxEndmonth.Text + "/" + textBoxEndYear.Text;
    nameDetailsOnCard = textBoxNameOnCard.Text;
    cardCVC= textBoxCVC.Text;

    DialogResult = DialogResult.OK;

    if (string.IsNullOrEmpty(textBoxName.Text) ||
        string.IsNullOrEmpty(textBoxCardNum.Text) &&
        string.IsNullOrEmpty(textBoxStartMonth.Text) ||
        string.IsNullOrEmpty(textBoxStartYear.Text) ||
        string.IsNullOrEmpty(textBoxEndmonth.Text) ||
        string.IsNullOrEmpty(textBoxEndYear.Text) ||
        string.IsNullOrEmpty(textBoxNameOnCard.Text) ||
        string.IsNullOrEmpty(textBoxCVC.Text))
    {
        buttonSave.Enabled = false;
    }
    else
    {
        buttonSave.Enabled = true;
    }
}

Answer 1

You save first

 DialogResult = DialogResult.OK;

then validate and trying to act:

if (...)
{
    buttonSave.Enabled = false;
}

Let's extract a method for validation:

// Let's be more friendly and show to user which control has invalid value
private Control InvalidControl() {
  //TODO: Add more conditions here - if year is an integer in 1990..2100 range etc.
  if (string.IsNullOrEmpty(textBoxName.Text))
    return textBoxName;
  if (string.IsNullOrEmpty(textBoxCardNum.Text) && 
      string.IsNullOrEmpty(textBoxStartMonth.Text))
    return textBoxCardNum;  
  if (string.IsNullOrEmpty(textBoxStartYear.Text))
    return textBoxStartYear; 
  if (string.IsNullOrEmpty(textBoxEndmonth.Text))
    return textBoxEndmonth; 
  if (string.IsNullOrEmpty(textBoxEndYear.Text))
    return textBoxEndYear; 
  if (string.IsNullOrEmpty(textBoxNameOnCard.Text))
    return textBoxNameOnCard; 
  if (string.IsNullOrEmpty(textBoxCVC.Text))
    return textBoxCVC;

  return null;
}

Then we can save if and only if all controls are valid:

private void buttonSave_Click(object sender, EventArgs e) {
  Control failed = InvalidControl();

  if (failed != null) {  // we have an invalid control
    if (failed.CanFocus) // let user know which is it:
      failed.Focus();    // we can set keyboard focus on it

    // Let user have a message on what's going on
    MessageBox.Show("Invalid value", //TODO: put a better text here 
                     Application.ProductName, 
                     MessageBoxButtons.OK, 
                     MessageBoxIcon.Warning);
  }
  else // Validation is OK, we can close the dialog with "OK" result 
    DialogResult = DialogResult.OK; 
}

If you want enable / disable the buttonSave you can implement TextChanged event handler for all the controls of interest: textBoxName, textBoxCardNum, ..., textBoxCVC and there

private void Textboxes_TextChanged(object sender, EventArgs e) {
  // we can save if and only if we have no invalid control(s)
  buttonSave.Enabled = InvalidControl() == null; 
}

Answer 2

One possible solution is to wire up (subscribe) all the text boxes TextChanged event to the same event. Then when the text changes in one of the text boxes, the event will fire. In the event, you could check each text box to see if they all have some text, and if they do, then enable the save button, otherwise, disable the save button.

Example, to wire up four (4) text boxes to the same TextChanged event may look something like…

textBox1.TextChanged += new EventHandler(textBox_TextChanged);
textBox2.TextChanged += new EventHandler(textBox_TextChanged);
textBox3.TextChanged += new EventHandler(textBox_TextChanged);
textBox4.TextChanged += new EventHandler(textBox_TextChanged);

Then to help, a List<TextBox> may come in handy to loop though only the text boxes we want. When the form loads, the code can add the text boxes to this list, something like…

TextBoxes = new List<TextBox>();
TextBoxes.Add(textBox1);
TextBoxes.Add(textBox2);
TextBoxes.Add(textBox3);
TextBoxes.Add(textBox4);

Then finally, do all the comparisons in the tetxBox_TextChanged event. This may look something like…

List<TextBox> TextBoxes;

private void Form2_Load(object sender, EventArgs e) {
  btnSave.Enabled = false;
  textBox1.TextChanged += new EventHandler(textBox_TextChanged);
  textBox2.TextChanged += new EventHandler(textBox_TextChanged);
  textBox3.TextChanged += new EventHandler(textBox_TextChanged);
  textBox4.TextChanged += new EventHandler(textBox_TextChanged);

  TextBoxes = new List<TextBox>();
  TextBoxes.Add(textBox1);
  TextBoxes.Add(textBox2);
  TextBoxes.Add(textBox3);
  TextBoxes.Add(textBox4);
}


private void textBox_TextChanged(object sender, EventArgs e) {
  foreach (TextBox tb in TextBoxes) {
    if (string.IsNullOrEmpty(tb.Text)) {
      btnSave.Enabled = false;
      return;
    }
  }
  btnSave.Enabled = true;
}

With this approach the save button will get enable when all the text boxes have some text and will be disabled is any text box is empty.

Answer 3

This is much more efficient way when checking all the textboxes than using messy comparison operator

List<TextBox> txts = panelName.Controls.OfType<TextBox>(); //just change the 'panelName' whether all your textboxes are located inside a form or just a panel.
foreach(TextBox txt in txts)
{
    if(string.IsNullOrEmpty(txt.Text)
        buttonSave.Enabled = false;
    else
        buttonSave.Enabled = true;
}