Insert data into database in ASP.NET throwing exception

Question

I'm trying to insert data into database in ASP.NET with this code:

string conn = "TJLDatabaseConnectionString";
conn = ConfigurationManager.ConnectionStrings["Conn"].ToString();
SqlConnection objsqlconn = new SqlConnection(conn);
objsqlconn.Open();
SqlCommand objcmd = new SqlCommand("Insert into MeterReading(MachineName,LastReading,CurrentReading,Consumption) Values('" + TextBox1.Text + "','" + TextBox2.Text + "','" + TextBox3.Text + "','" + TextBox4.Text + "')", objsqlconn);
objcmd.ExecuteNonQuery();
//MessageBox.Show("Successful");

But when I run it. It gives the following message:

• Tadit Dash: You forgot to tell the Exception message.
• Murali Murugesan: It gives following message What is it?
• Richard: NB you are open to a SQL injection attack: never combine values into SQL statements with string concatenation. Use parametrised queries (objcmd.Paramters.Add(name, value)).
• ps2goat: it looks like one of your values may have an unescaped ', making the query code and string/varchar values mix together.
• The Hungry Dictator: what is the definition of TJLDatabaseConnectionString??

Answer 1

First the important, always use sql-parameters to prevent sql-injection. Never concatenate parameters into a sql-query. This can also solve localization or "escaping" issues.

Also, use the using statement to ensure that anything using unmanaged resources (like a sql-connection) will be closed and disposed even on error:

string sql = @"
INSERT INTO MeterReading(MachineName,LastReading,CurrentReading,Consumption) 
VALUES(@MachineName,@LastReading,@CurrentReading,@Consumption)";
using(var objsqlconn = new SqlConnection(ConfigurationManager.ConnectionStrings["Conn"].ToString()))
using (var cmd = new SqlCommand(sql, objsqlconn))
{
 cmd.Parameters.AddWithValue("@MachineName", TextBox1.Text);
 cmd.Parameters.AddWithValue("@LastReading", TextBox2.Text);
 cmd.Parameters.AddWithValue("@CurrentReading", TextBox3.Text);
 cmd.Parameters.AddWithValue("@Consumption", TextBox4.Text);
 objsqlconn.Open();
 int insertedCount = cmd.ExecuteNonQuery();
}

Side-note: if you have an identity column and you want to retrieve the newly created primary-key, use SCOPE_IDENTITY and ExecuteScalar even if you use INSERT INTO:

string sql = @"
INSERT INTO MeterReading(MachineName,LastReading,CurrentReading,Consumption) 
VALUES(@MachineName,@LastReading,@CurrentReading,@Consumption); 
SELECT CAST(scope_identity() AS int)";
//...
int newID = (int)cmd.ExecuteScalar();

Answer 2

Use a variable to check if row is getting affected or not

 rowAffected= objcmd.ExecuteNonQuery();
 if(rowAffected >0)
 {
 //sucessful
 }
 else
 {
 //
 }

Answer 3

Since there is no any exception mention in your question so just for a better and readable code I would suggest you too use using blocks. It gives you nice, cleaner and readable code and also handle objects when they go out of scope.

This is meant for good practices that we generlly follow while coding. Kindly show us the exception for appropriate solution.

private void ConnectToDb()
{
var conn = ConfigurationManager.ConnectionStrings["Conn"].ConnectionString;
using( var conn = new SqlConnection(conn))
{
 conn.Open();
 var cmdtxt ="Insert into MeterReading(MachineName,LastReading,CurrentReading,Consumption) 
Values(@P1,@P2,@P3,@P4)";
 using(var cmd = new SqlCommand(cmdtxt, conn))
 {
 cmd.CommandType=CommandType.Text;
 cmd.Parameters.AddWithValue("@P1", TextBox1.Text);
 cmd.Parameters.AddWithValue("@P2", TextBox2.Text);
 cmd.Parameters.AddWithValue("@P3", TextBox3.Text);
 cmd.Parameters.AddWithValue("@P4", TextBox4.Text);
 cmd.ExecuteNonQuery();
 }
 con.close();
}
}

• Steve: You deserve an upvote, but you still fail to explain why your answer resolves the OP problem.
• user240141: @Steve , Still OP has not provided the actual exception .. So I have just suggested him to go for such code might be his issue will get fixed..
• Steve: You have made another substantial change to the original OP code, and I bet that it is this change that has the bigger probability to fix the OP code. But you don't explain it, this is the meaning of my comment.