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?
 
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
 
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
Cloud Class® Course: Microsoft Exchange Server

The MCTS: Microsoft Exchange Server 2010 certification validates your skills in supporting the maintenance and administration of the Exchange servers in an enterprise environment. Learn everything you need to know with this course.

 
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
 
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
 
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
Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.

All Courses

From novice to tech pro — start learning today.