What am I doing wrong?
You are not executing the command.
Your code should have a cmd.ExecuteNonQuery(); once the command is fully configured.
However, once you add that, you will get a syntax error message from SQL Server.
This is because you are missing a closing parenthesis for the Exists operator.
Please note that is far from being the only thing you are doing wrong in this code:
- you are mixing UI code with application code.
- you are using a global variable (or at the very least a field) to hold your instance of the
SqlConnection.
- you are using terrible names for your parameters and texboxes.
- You are using
AddWithValue.
- the pattern for "upsert" you are using is cumbersome and potentially problematic.
Here are better alternatives:
First
You should read about the n-tier architectural pattern.
For winforms, it's usually implemented using MVP.
For one thing, instead of sending textboxes data directly into your database, create a Customr class to hold the data, and use that to pass customer data around in your code.
Second
Best practice is to use a local variable inside a using statement to ensure disposale of the SqlConnectioninstance and the return of the underlying connection to the connection pool.
Third
Imagine having to change something in a code that looks like this vs. changing something in a code that looks like that:
cmd.Parameters.Add(@"CustomerName", SqlDbType.NVarChar).Value = customerName;
Now you don't have to read at the SQL to figure out what that parameter means -
the less time and effort you have to spend understanding the code the better.
Fourth
The article in the link explains why it's problematic in details,
but the main point is that the data type of the parameter must be inferred from usage,
and that might yield errors because of data type wrongly inferred - or even worst - wrong data silently entered into the database.
Fifth
A better pattern is to first update, and then insert conditionally - like demonstrated in Aaron Bertrand's answer here - and in a multi-user (or multi-threaded) environment wrap the entire thing in a transaction.
All that being said, a revised code should look more like this:
private void AddOrUpdateCustomer(Customer customer)
{
// Data validity tests omitted for brevity - but you should ensure
// customer has all it's properties set correctly.
// Readable, properly indented code - Isn't that much easier to debug?
var sql = @"
SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
BEGIN TRANSACTION;
UPDATE [Customers]
SET
Cellphone_Number = @Cell,
Telephone_Number = @Telephone,
Alternative_Number = @Alternative
WHERE Customer_Name = @Name
IF @@ROWCOUNT = 0
BEGIN
INSERT INTO [Customers](Customer_Name, Cellphone_Number, Telephone_Number, Alternative_Number)
VALUES(@Name, @Cell, @Telephone, @Alternative)
END
COMMIT TRANSACTION;";
// connectionString should be obtained from configuration file
using(var con = new SqlConnection(connectionString))
{
using(var cmd = new SqlCommand(sql, con))
{
cmd.Parameters.Add(@"Name", SqlDbType.NVarChar).Value = customer.Name;
cmd.Parameters.Add(@"Cell", SqlDbType.NVarChar).Value = customer.Cellphone;
cmd.Parameters.Add(@"Telephone", SqlDbType.NVarChar).Value = customer.Telephone;
cmd.Parameters.Add(@"Alternative", SqlDbType.NVarChar).Value = customer.AlternativeNumber;
con.Open();
cmd.ExecuteNonQuery();
}
}
}
