1

I am starting a project and I have an Onclick function running in the code behind of my web form. I was curious if there was a way to make the code shorter and/or faster?

protected void PageSubmit_Click(object sender, EventArgs e)
{
    string storedProc = "";
    string successMsg = "";

    DataTable dt = new DataTable();
    if (hfPageID.Value.Length > 0)
    {
        storedProc = "pageUpdate";
        successMsg = "Page updated!";
    }
    else
    {
        storedProc = "pageInsert";
        successMsg = "Page inserted!";
    }
    using (SqlConnection con = new SqlConnection(Global.conString))
    {
        con.Open();
        using (SqlCommand cmd = new SqlCommand(storedProc, con))
        {
            if (storedProc == "pageUpdate")
            {
                cmd.Parameters.Add("@originalID", SqlDbType.VarChar).Value = hfPageID.Value;
            }
            cmd.Parameters.Add("@title", SqlDbType.VarChar).Value = Global.SafeSqlLiteral(txtPage.Text, 1);
            cmd.Parameters.Add("@contentTypeID", SqlDbType.VarChar).Value = rblContentTypesGetAll.SelectedValue;
            cmd.CommandType = CommandType.StoredProcedure;
            cmd.ExecuteNonQuery();
        }
        con.Close();

        //Update Content Page Repeater
        using (SqlCommand cmd = new SqlCommand("pageGetAll", con))
        {
            using (SqlDataAdapter da = new SqlDataAdapter(cmd))
            {
                da.Fill(dt);
            }
        }
    }

    Session["formProcessed"] = successMsg;
    Response.Redirect(redirectURL);
}

Also, I set up a check in the stored procedure to fail the pageUpdate if a title conflicts with an existing title. How can I check if it actually updated when it's coming back from the stored procedure?

ALTER PROCEDURE [dbo].[pageUpdate]
    @originalID uniqueidentifier,
    @contentTypeID varchar (100),
    @title varchar (100)
AS 

UPDATE 
    pages
SET
    contentTypeID = COALESCE (@contentTypeID, contentTypeID),
    title = COALESCE (@title, title)
WHERE
    ID = @originalID
    AND 
        NOT EXISTS
        (
            SELECT
                *
            FROM
                pages
            WHERE
                title = @title
        )
balexander
  • 23,131
  • 14
  • 45
  • 68

1 Answers1

3

There's a number of things you could do that would certainly help readability and code reuse.

Seperate your page/UI logic from the database logic. i.e. have a class that handled data access and call that from your page rather than have it on the click event. If you wanted this data elsewhere, you'd have to copy and paste the code rather than just call the same method on a common object.

Furthermore to cut down your database code, you could look at using a Data Access Application Block as featured in the Enterprise Library like these here. It'll handle connections and exceptions for you amongst other benefits listed in the documentation.

Fellmeister
  • 591
  • 3
  • 24