3
\$\begingroup\$

I'm new to C#. I have created a form that has 5 text boxes and this function checks to see if every text box has text in the box. If not, then it displays a message box saying "Error (Add Text)".

I wanted to see if there was a way to simplify this, or if this is the best way of completing this task :)

    private bool ValidInput()
    {
        bool isValid = true;
        //Check Title
        if (string.IsNullOrEmpty(titleText.Text))
        {
            //Message Box Pass Through Title
            MB("The Title can't be blank!", "Error!", MessageBoxIcon.Error);
            titleText.Focus();
            isValid = false;
        }
        //Check Artist
        else if (string.IsNullOrEmpty(artistText.Text))
        {
            //Message Box Pass Through Artist
            MB("The Artist can't be blank!", "Error!", MessageBoxIcon.Error);
            artistText.Focus();
            isValid = false;
        }
        //Check Genre
        else if (string.IsNullOrEmpty(genreText.Text))
        {
            //Message Box Pass Through Genre
            MB("The Genre can't be blank!", "Error!", MessageBoxIcon.Error);
            genreText.Focus();
            isValid = false;
        }
        //Check Year
        else if (string.IsNullOrEmpty(yearText.Text))
        {
            //Message Box Pass Through Year
            MB("The Year can't be blank!", "Error!", MessageBoxIcon.Error);
            yearText.Focus();
            isValid = false;
        }
        //Check URL
        else if (string.IsNullOrEmpty(urlText.Text))
        {
            //Message Box Pass Through URl
            MB("The URL can't be blank!", "Error!", MessageBoxIcon.Error);
            urlText.Focus();
            isValid = false;
        }
        return isValid;
    }
\$\endgroup\$
-1
\$\begingroup\$

Try something like this:

private bool ValidInput()
{
     bool isValid = true;
     List<string[]> controlList = new List<string[]>(); //Control name, then display name.

     controlList.Add(new[] { "titleText", "Title"});
     controlList.Add(new[] { "artistText", "Artist" });
     controlList.Add(new[] { "genreText", "Genre" });
     controlList.Add(new[] { "yearText", "Year" });
     controlList.Add(new[] { "urlText", "URL" });

     for (int i = 0; i < controlList.Count; i++)
     {
         if (string.IsNullOrEmpty(this.Controls[controlList[i][0]].Text))
         {
            MB("The " + controlList[i][1] + " can't be blank!", "Error!", MessageBoxIcon.Error);
            this.Controls[controlList[i][0]].Focus();
            isValid = false;
            break;
         }
     }
     return isValid;
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. \$\endgroup\$ Nov 16 '21 at 18:00
  • \$\begingroup\$ This one works amazingly! Because it allows for more then just TextBox's so if I change one of the box's to a ComboBox it still works :D Thank you! \$\endgroup\$
    – Ablac
    Nov 17 '21 at 19:02
2
\$\begingroup\$

you can use Dictionary to store the TextBoxs and their representive name, then just iterate over them, something like this :

private bool ValidInput()
{
    Dictionary<string, TextBox> textBoxes = new Dictionary<string, TextBox>
    {
        { "Title", titleText },
        { "Artist", artistText },
        { "Genre", genreText },
        { "Year" , yearText},
        { "URL", urlText }
    };

    foreach(var item in textBoxes)
    {
        var name = item.Key;
        var textbox = item.Value;

        if (string.IsNullOrEmpty(textbox.Text))
        {
            MB($"The {name} can't be blank!", "Error!", MessageBoxIcon.Error);
            textbox.Focus();
            return false;
        }
    }


    return true;
}
\$\endgroup\$
2
\$\begingroup\$

Each Control does have a property called Tag. This can contain/hold any kind of information since its type is object.

  1. Assign the Title, ... ,Url values to the corresponding TextBox's Tag:
private readonly TextBox[] textBoxes = new [] { titleText, ... , urlText };
public Form1()
{
   InitializeComponent();
   titleText.Tag = "Title";
   ...
   urlText.Tag = "URL";
}
  1. Rewrite your ValidInput like this:
var textbox = textBoxes.FirstOrDefault(tb => string.IsNullOrEmpty(tb.Text));
if (textbox == null) return true;

MB($"The {textbox.Tag} can't be blank!", "Error!", MessageBoxIcon.Error);
textbox.Focus();
return false;
\$\endgroup\$
2
\$\begingroup\$

.net already provides a mechanism to validate TextBoxes and other controls. It's called the Validating event. This works in combination with the ErrorProvider that allows you to easly show an error message.

By wiring and handling this event you don't need any dictionaries, loops, arrays and alike.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.