HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

In need of some aid in regards to making my code more efficient

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
needmoreefficientsomecodemakingregardsaid

Problem

I've created a form that gathers and submits information to SQL database via LINQ, and sends an email if all goes while saving the database and doing a redirect. Recently, it started submitting data and calling errors (thread aborts) within the submit function, which seemed unusual. I solved it by using Boolean flags, and all works well. Looking over my code now, I am quite sure I can make it more efficient. I am seeking advice on this.

My biggest concern is handling the exceptions correctly, and not having unnecessary code, but efficient code.

Below is a basic overview of the backend of my form:

```
public partial class myClass : System.Web.UI.UserControl
{
private regEntities _reg = new regEntities();
private const string _formName = "Form Name";
private Boolean dbError = false;
private Boolean transError = false;
private Exception ce;
private Exception st;

protected void Page_Load(object sender, EventArgs e)
{
if (!Page.IsPostBack)
{
Initialize();
}

Session["FormName"] = _formName;
}
private void Initialize()
{
FormHelpers.GenerateStates(ddlState);
FormHelpers.GenerateCountries(ddlCountry);
}
private void CreateEntry()
{
try
{
diploma dip = new diploma();

dip.ssn = txtStudID.Text;
dip.fname = txtFirstName.Text;
dip.mname = txtMi.Text;
dip.lname = txtLastName.Text;
dip.city = txtCity.Text;
dip.state = ddlState.SelectedItem.Text;
dip.country = ddlCountry.SelectedItem.Text;
dip.email = txtEmail.Text;
dip.grad_year = txtGYear.Text;
dip.grad_term = ddlGterm.SelectedItem.Text;
dip.degree = ddlSDegree.SelectedItem.Text;
dip.submit_date = DateTime.Now;

_registrar.AddTotables(dip);
_registrar.SaveChanges();

}
catch (Exception oe)
{

Solution

Is there any reason you can't do it the easy way?

protected void FormSubmit(object sender, EventArgs e)
    {
    Page.Validate();
    if (!Page.IsValid)
        return;

    try
        {
        CreateEntry();
        SendAdminMail("email@email.com", txtFirstName.Text + " " + txtLastName.Text);
        Response.Redirect("/thank-you", true);
        }
    catch (Exception ex)
        {
        ThrowDbError(ex);
        Response.Redirect("/error", true);
        }
    }


If you want to handle certain exceptions more specifically, the "right" way to do it is with additional catch blocks for the exceptions which need special handling (e.g., SqlException). Offhand I don't see why you'd want to; since this is a web app, my inclination would be to dump any exception to a log (or e-mail it to an admin) but offer only generic output to the user, to avoid exposing implementation details.

I may be missing something. Was there a specific reason you used that bool-and-if pattern?

Code Snippets

protected void FormSubmit(object sender, EventArgs e)
    {
    Page.Validate();
    if (!Page.IsValid)
        return;

    try
        {
        CreateEntry();
        SendAdminMail("email@email.com", txtFirstName.Text + " " + txtLastName.Text);
        Response.Redirect("/thank-you", true);
        }
    catch (Exception ex)
        {
        ThrowDbError(ex);
        Response.Redirect("/error", true);
        }
    }

Context

StackExchange Code Review Q#29794, answer score: 2

Revisions (0)

No revisions yet.