Proper way of storing RadioButton values in DB

Question

Hi All, I need some suggestions on following code:

Scenario: Its like a Opinion poll, You have to choose Yes/No and the data will be stored on DB.But I think this code can be optimized,can some body suggest proper way of doing this.

<form id="form1" runat="server">
  <div>
    <asp:radiobuttonlist id="RadioButtonList1" runat="server">
      <asp:ListItem Text="yes" Value="1"></asp:ListItem>
      <asp:ListItem Text="No" Value="0"></asp:ListItem>
    </asp:radiobuttonlist>
  </div>
  <div>
    <asp:button id="Button1" runat="server" text="Submit" onclick="Button1_Click" />
  </div>
</form>

protected void Button1_Click(object sender, EventArgs e)
{
  if (RadioButtonList1.Items.FindByValue("1").Selected == true)
  {
    string source = "Server=localhost;Database=Test;Trusted_Connection=yes";
    SqlConnection con = new SqlConnection(source);
    con.Open();
    SqlCommand cmd = new SqlCommand("proc_select", con);
    SqlDataReader dr = cmd.ExecuteReader();
    if (dr.HasRows==true)
    {
      while (dr.Read())
      {
        int val = (int)dr["yes"];
        val++;
        update(val);
      }
    }
  }

  if (RadioButtonList1.Items.FindByValue("0").Selected == true)
  {
    string source = "Server=localhost;Database=Test;Trusted_Connection=yes";
    SqlConnection con = new SqlConnection(source);
    con.Open();
    SqlCommand cmd = new SqlCommand("proc_select", con);
    SqlDataReader dr = cmd.ExecuteReader();
    if (dr.HasRows == true)
    {
      while (dr.Read())
      {
        int val = (int)dr["No"];
        val++;
        updateNo(val);
      }
    }
  }
}

public void update(int val)
{
  string source = "Server=localhost;Database=Test;Trusted_Connection=yes";
  SqlConnection con = new SqlConnection(source);
  SqlCommand cmdupd;
  con.Open();
  cmdupd = new SqlCommand("proc_Update", con);
  cmdupd.Parameters.Add("@val", SqlDbType.NVarChar).Value = val;
  cmdupd.CommandType = CommandType.StoredProcedure;
  cmdupd.ExecuteNonQuery();
}

UpdateNo() method will be same as update().

I will declare sqlcommand and source like things globally, so not on that area. Let me know if u want more information.

Answer 1

You did not make your question specific enough (what kind of suggestions do you want?, what kind of optimization are you looking for?), so my answer is general in nature:

I think your sample could surely qualify for the "Code Duplicacy award", if not the kind of code we see on Coding Horror. I cannot believe that you cannot see the amount of duplicated code here!

You want suggestions. I will give you a few questions to ponder...

For instance:

• Why have two separate if constructs if the only difference within them is the value of the column (yes/no) and the method call?
• Even in the current state, why did you not use if-else? (these are RadioButtons!)
• Why is the database access code (connection creation, command creation, etc.) repeated at every step? Why is it not extracted into a separate function?
• Why have two separate Update functions when the only difference is the column name to be updated?
• What is the use of retrieving the value from the database to your application, incrementing it and then running an Update statement to update the value in the database? Is it not possible to create a stored procedure that directly increments and updates a value in a particular column?
• The list would go on... but I think I'm going to get some ice-cream just to change the taste in my mouth.

Answer 2

hi, with things like this you would be better going down a path of jquery and a generic HttpHandler

<select name="" id="" onchange="makepost(this.value);">
<option value="">Yes/No</option>
<option value="0">No</option>
<option value="1">Yes</option>
</select>

<script>
function makepost(myanswer)
{
$.ajax({
url:"ajax/processanswer.ashx",
data: "answer=" + myanswer,
success: function(){location.href=("mypage.aspx");},
error: function(){alert("Post Failed, please try later");}

});
}
</script>

then your ashx will simply collect your answer and update the database

public void ProcessRequest(Httpcontext ctx)
{
var myanswer = ctx.request["answer"]; //add cleanup function, checking etc

// add code here to update database with answer

and that should be it really
}