Correct way to store RadioButton values in DB
I need suggestions for the following code:
Scenario: . Like the survey, you have to select Yes / No and the data will be stored in DB.But I think this code can be optimized if some body can suggest the correct 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()
will be the same as update()
.
I will declare sqlcommand
and source as things all over the world, therefore not in this area. Let me know if you need more information.
a source to share
with things like this you are better off going the jquery and generic HttpHandler path
<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 just 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
}
a source to share
You haven't made your question specific enough (what suggestions do you want?), What kind of optimization are you looking for?), So my answer is general:
I think that your sample can certainly qualify for the " Diplicacy Code Award " if not the code we see on Coding Horror . I can't believe you can't see the amount of duplicate code here!
You want suggestions. I'll give you some questions to ponder ...
For instance:
- Why are there two separate constructs
if
when the only difference within them is the column value (yes / no) and the method call? - Even in its current state, why didn't you use it
if-else
? (these are RadioButtons!) - Why is the database access code (creating a connection, creating a command, etc.) repeated at every step? Why isn't it being extracted into a separate function?
- Why are there two separate update functions when the only difference is the name of the column to update?
- What is the use of fetching a value from the database into an application, incrementing it, and then running an Update statement to update the value in the database? Can't create a stored procedure that directly increments and updates a value in a specific column?
- The list will go on ... but I think I'm going to get ice cream to change the taste in my mouth.
a source to share