CODEWITHSUNDEEP
c#methods
marcmiller2007
marcmiller2007

Reputation: 119

button click event C#

So I have a Windows form with a few fields to accept data and write to a SQL database. My button_click event is what handles all the work except for validating the user input. These are handling by separate methods. My question is that if a user has incorrect input, how do I reset the form after showing the messagebox? Maybe just use the button click event to start a separate method so I can control it better?

private void enter_button_Click(object sender, EventArgs e)
{
    restart:
    try {
        try {
                Validate(fname);
                Validate(lname);
                Validate(city);
                Validate(state);
        } catch (Exception ex) {
            MessageBox.Show(ex.Message);
            if (ex != null) {
                fname.Clear();
                lname.Clear();
                city.Clear();
                state.Clear();
                goto restart;
            }
        }
        try {
            exValidate(address);
        } catch (Exception ex1) {
            MessageBox.Show(ex1.Message);
            if (ex1 != null) {
                address.Clear();
                goto restart;
            }
        }
        //blah blah...write to database.
    }
    // ...
}

Upvotes: 1

Views: 1711

Answers (5)

user1162770
user1162770

Reputation: 3

if (MessageBox.Show(Exception.Message) == System.Windows.Forms.DialogResult.OK)
{
    //Clear all needed controls here
}

Upvotes: 0

GeorgePotter
GeorgePotter

Reputation: 899

My suggestion would be use a background worker thread to handle the logic and just have the background worker triggered by button_click. That way you can terminate the background worker thread if the user makes a mistake and restart the whole process via button_click.

Sources:

how to keep a control disabled till a thread ends

http://msdn.microsoft.com/en-us/library/system.componentmodel.backgroundworker.aspx

Upvotes: 0

Adolfo Perez
Adolfo Perez

Reputation: 2874

You have too much logic in the button click. Personally I would rather validate on lost focus of each field. If you dont' want to do that then create a method like bool ValidateForm() and wrap all your validation logic there. If at least one inner validation fails then return false. Also create a method like void ClearForm() where you wrap all the logic to clear all fields. Remember that is always good to modularize your code so you can reuse your logic, here you are tying the clear and validation logic to a particular event (button click).

So on Enter Click you want to

if(!ValidateForm())
{
   MessageBox.Show("Invalid Form");
   ClearForm();
   return;
}
SaveForm();

I would highly recommend not using GOTO instructions AT ALL!! This is a very dangerous practice that can turn your code into a spaghetti nightmare.

Hope this helps.

Upvotes: 2

Olivier Jacot-Descombes
Olivier Jacot-Descombes

Reputation: 112352

Just drop the gotos. And let the user fix the wrong entries. When he has done it, he will click on the button again. If you go to restart: you might be caught in an endless loop.

If you clear all the fields when an error occurs, the user will have to reenter all the fields. Just tell him which fields are wrong and why, so that he can just fix the bad entries and can click the button when he fixed the problems.

And by the way, using goto is really ugly. Use it only in very very rare situations!

Upvotes: 0

Ryan
Ryan

Reputation: 1797

Lots of problems with your code. Why do you check if ex!=null after you display it in the MessageBox? And using goto labels is not standard in C#.

The nested try is not nice either. Seperate your code a bit, as in have a seperate function that takes care of just validating the data. Your button click handler should not have to do all this work. So you would have a validate() function that takes care of validation logic and a clearForm().

That would essentially untangle all the mess in your button click handler and it would simply have to do something along the lines of:

if (validate())
  //save to db
else
  clearForm()

Also you should rethink your design, function should only throw exception if it is really needed. Do you need to throw an exception because one form input was not in a valid format?

Upvotes: 0

Related Questions