Try - catch code: is this correct

I have this try-catch code below but not sure where I actually should put the try-catch.
I have a "save" method and a button and I have the try catch in both places.

Note that the try catch in "button" click is the one that gets called when i have an error.

is this correct? should I move the other 2 catch statements?
protected void btnTestSave_Click(object sender, EventArgs e)
    {
        if (!Page.IsValid)
        {
        }
        else
        {
            try
            {
                Save();
                Response.Redirect("thank-you.aspx",false); //payment will go here
            }
            catch (Exception ex)
            {
                lblMsg.Text = "An error occured. Please contact Retina Health";
                Common common = new Common();
                common.Log("New user", "Signup", string.Concat(ex.Message, "Stack Trace Starts here", ex.StackTrace, "inner exception starts here: ", ex.InnerException));
            }
        }
    }

-----------

 private void Save()
    {
        using (TransactionScope scope = new TransactionScope())
        {
            try
            {
                Sign_UpData sd = new Sign_UpData();
                int businessNameId = SaveDescription(sd);
                SaveContactPerson(sd, businessNameId);
                SaveOfficeLocation(sd, businessNameId);
                SaveHCProviderInformation(sd,businessNameId);
                SaveOfficerUserName(sd, businessNameId);

                scope.Complete();
            }

            catch (TransactionAbortedException ex)
            {
                Common common = new Common();
                common.Log("New user", "Signup", string.Concat(ex.Message, "Stack Trace Starts here:", ex.StackTrace, "inner exception starts here: ", ex.InnerException ));
            }
            catch (ApplicationException ex)
            {
                Common common = new Common();
                common.Log("New user", "Signup", string.Concat(ex.Message, "Stack Trace Starts here", ex.StackTrace, "inner exception starts here: ", ex.InnerException));
            }



        }

    }

Open in new window

LVL 8
CamilliaAsked:
Who is Participating?

[Product update] Infrastructure Analysis Tool is now available with Business Accounts.Learn More

x
I wear a lot of hats...

"The solutions and answers provided on Experts Exchange have been extremely helpful to me over the last few years. I wear a lot of hats - Developer, Database Administrator, Help Desk, etc., so I know a lot of things but not a lot about one thing. Experts Exchange gives me answers from people who do know a lot about one thing, in a easy to use platform." -Todd S.

Kiran SonawaneProject LeadCommented:
You can remove try catch block in btnTestSave_Click event and

 try
            {
                Sign_UpData sd = new Sign_UpData();
                int businessNameId = SaveDescription(sd);
                SaveContactPerson(sd, businessNameId);
                SaveOfficeLocation(sd, businessNameId);
                SaveHCProviderInformation(sd,businessNameId);
                SaveOfficerUserName(sd, businessNameId);

                scope.Complete();
            }

            catch (TransactionAbortedException ex)
            {
                Common common = new Common();
                common.Log("New user", "Signup", string.Concat(ex.Message, "Stack Trace Starts here:", ex.StackTrace, "inner exception starts here: ", ex.InnerException ));
            }
           catch (Exception ex)
            {
                lblMsg.Text = "An error occured. Please contact Retina Health";
                Common common = new Common();
                common.Log("New user", "Signup", string.Concat(ex.Message, "Stack Trace Starts here", ex.StackTrace, "inner exception starts here: ", ex.InnerException));
            }
0
käµfm³d 👽Commented:
The reason the try/catch in the button is called is because your Save method is throwing an exception other than TransactionAbortedException or ApplicationException. Since you used the general Exception in your button click hander (which is highly discouraged), you will catch any uncaught exceptions that occur with the corresponding try.

You should have a catch for each potential exception that can occur within a give try. If you want the code which calls the function to handle the exception instead of the function where the exception actually occurred, then rethrow the exception. This at least indicates to others (and you at some later date) that you expected an exception of type XXX to occur, but you intended on the calling code to handle the exception.

Here is an example of what I mean:

protected void btnTestSave_Click(object sender, EventArgs e)
{
	if (!Page.IsValid)
	{
	}
	else
	{
		try
		{
			Save();
			Response.Redirect("thank-you.aspx",false); //payment will go here
		}
		catch (FormatException ex)  // I am handling any FormatExceptions which might occur in Save()
		{
		    lblMsg.Text = "Invalid format received.";
		}
		catch (Exception ex)
		{
			lblMsg.Text = "An error occured. Please contact Retina Health";
			Common common = new Common();
			common.Log("New user", "Signup", string.Concat(ex.Message, "Stack Trace Starts here", ex.StackTrace, "inner exception starts here: ", ex.InnerException));
		}
	}
}

-----------

private void Save()
{
	using (TransactionScope scope = new TransactionScope())
	{
		try
		{
			Sign_UpData sd = new Sign_UpData();
			int businessNameId = SaveDescription(sd);
			SaveContactPerson(sd, businessNameId);
			SaveOfficeLocation(sd, businessNameId);
			SaveHCProviderInformation(sd,businessNameId);
			SaveOfficerUserName(sd, businessNameId);

			scope.Complete();
		}

		catch (TransactionAbortedException ex)
		{
			Common common = new Common();
			common.Log("New user", "Signup", string.Concat(ex.Message, "Stack Trace Starts here:", ex.StackTrace, "inner exception starts here: ", ex.InnerException ));
		}
		catch (FormatException ex)
		{
		    throw;  // I am going to pass on the exception for tha caller to handle
		}
		catch (ApplicationException ex)
		{
			Common common = new Common();
			common.Log("New user", "Signup", string.Concat(ex.Message, "Stack Trace Starts here", ex.StackTrace, "inner exception starts here: ", ex.InnerException));
		}
	}

}

Open in new window


In the above, I used FormatException as an example. I rethrew the exception by using the throw keyword. This allows the caller to handle the exception. I handled this exception by printing a message to the label.
0
CamilliaAuthor Commented:
kaufmed -
1. Should I even have try-catch in btnTestSave_Click method? should I move all try-catch to "Save"?
If I keep it as is, will try-catch in "btnTestSave_click" ever be fired?

2. Can I have an "error" class instead of having 10 whatever "catch" section in this section of the code?

3. Is "innerException" i have there correct to log exactly where the error has occured?
0
Angular Fundamentals

Learn the fundamentals of Angular 2, a JavaScript framework for developing dynamic single page applications.

santhimurthydCommented:
My recommendation will be like this

There are experts who already discussed on this topic and they can give more info
protected void btnTestSave_Click(object sender, EventArgs e)
        {
            if (!Page.IsValid)
            {
            }
            else
            {
                string errmessage = errStackTrace = errInnermessage  = string.Empty;
                if(Save(out errmessage , out errStackTrace , out errInnermessage))
                {
                    Response.Redirect("thank-you.aspx",false); //payment will go here
                }   
                else
                {
                    lblMsg.Text = "An error occured. Please contact Retina Health";
                    Common common = new Common();
                    common.Log("New user", "Signup", string.Concat(errmessage, "Stack Trace Starts here", errStackTrace, "inner exception starts here: ", errInnermessage));
                }
            }
        }

        -----------

        private bool Save(out string errmessage , out string errStackTrace , out string errInnermessage)
        {
            errmessage = errStackTrace = errInnermessage  = string.Empty;
            using (TransactionScope scope = new TransactionScope())
            {
                try
                {
                    
                        Sign_UpData sd = new Sign_UpData();
                        int businessNameId = SaveDescription(sd);
                        SaveContactPerson(sd, businessNameId);
                        SaveOfficeLocation(sd, businessNameId);
                        SaveHCProviderInformation(sd,businessNameId);
                        SaveOfficerUserName(sd, businessNameId);

                        scope.Complete();
                        
                
                }

                catch (TransactionAbortedException ex)
                {
                    Common common = new Common();
                    errmessage = ex.Message;
                    errStackTrace = ex.StackTrace;
                    errInnermessage = ex.InnerException;
                    common.Log("New user", "Signup", string.Concat(ex.Message, "Stack Trace Starts here:", ex.StackTrace, "inner exception starts here: ", ex.InnerException ));
                    return false;
                }
                catch (ApplicationException ex)
                {
                    errmessage = ex.Message;
                    errStackTrace = ex.StackTrace;
                    errInnermessage = ex.InnerException;
                    Common common = new Common();
                    common.Log("New user", "Signup", string.Concat(ex.Message, "Stack Trace Starts here", ex.StackTrace, "inner exception starts here: ", ex.InnerException));
                    return false;
                }
                return true;
            }

        }

Open in new window

0
CamilliaAuthor Commented:
I dont need a class to have ALL the possible exceptions?
0
santhimurthydCommented:
It's always better to go with the specific exception instead of general exception as it will consume more memory than the specific exception and the information are very generic

e.g
FileNotfound
ArthimeticExcetion
SQLException
OverflowException

Are the exceptions you expected and have workaround to fix them.  Once the workaround has been identified then your application and the business will execute as expected.
If you go with generic exception the business execution have to aborted else will give an unexpected behaviors as you may do some workaround as this the exception will occur, but the exception occurred may be different.

e.g You though that some arithmetic exception will occur and it occur, you need to do some logic to continue but the exception occur may IO exception or some other than you expected, but the code will continue as the exception occurred as arithmetic .
0

Experts Exchange Solution brought to you by

Your issues matter to us.

Facing a tech roadblock? Get the help and guidance you need from experienced professionals who care. Ask your question anytime, anywhere, with no hassle.

Start your 7-day free trial
CamilliaAuthor Commented:
I have "Exception" which kaufmed also said is not good. What's the exception I should use when I get "input string was not in correct format" AND "binray or text will be truncated"

I've fixed those errors but still..what kind of an exception should I use? thanks
0
santhimurthydCommented:
Since you are connecting to SQL database, you can use the "SqlException" and "TransactionAbortedException" which will help to handle exception related to SQL connection,  Time out exception from SQL server  and transaction error occurred.

If any other exception occurred let it be occurred and get the detail of the exception from even viewer log as check whether any fix have to done to the code as you mentioned in your comment else handle it like "SqlException" and "TransactionAbortedException"
0
CamilliaAuthor Commented:
"input string was not in correct format"  this is an ASP.net error sometimes. For example, i have a string value but i did int.Parse(<that string value>). Should this be "Exception" try-catch?
0
santhimurthydCommented:
If your trying the int.Parse(<<string Value>>) inside an existing try{ ... } catch{} block, add the "InvalidCastException" to the exception list.

If the int.Parse is not inside an try{ .. } catch{} block, you can use the int.TryParse(<<string value>> , out intvalue) it will return an bool value indication whther the cast is valid and able to convert, if not then the return will be false and the OUT INTPARAM in the method will have 0"Zero" as value.
0
CamilliaAuthor Commented:
ah, good ideas. Thanks.
0
santhimurthydCommented:
Your welcome, Now i feel as you have got very good hands on handling exception in an project.
0
CamilliaAuthor Commented:
One last one, I know i closed this...i should've even use "Exception" (the general one) at all in the app?
0
santhimurthydCommented:
It's not restricted to use "Exception" in general. It's about the best practises of handling the exception.

E.g if you started working on busienss logic, you will not able to trace all the exception at the begining itself and start using general "Exception" and going on continous catch you will get this which specific exception will occur in this logic and that time you can revert back to specific then general make sure the application stable and remove the general exception and it's recommended to avoid using general "Exception" object until unless you not able to narrow down to the specific exception because it consumes more memory than specific exception object.
0
It's more than this solution.Get answers and train to solve all your tech problems - anytime, anywhere.Try it for free Edge Out The Competitionfor your dream job with proven skills and certifications.Get started today Stand Outas the employee with proven skills.Start learning today for free Move Your Career Forwardwith certification training in the latest technologies.Start your trial today
ASP.NET

From novice to tech pro — start learning today.