0

I have a table named "Members" in my database.

In a form, I have some Labels, TextBoxes, a DataGridView and a few Buttons.

When I click a row in the DataGridView, the whole data appears in the corresponding TextBox correctly.

I have added an "Update" button - which, when clicked I want the database table information to update instantly.

I have written the following code on "Update" button click event. But it shows me error continuously. Please help. I badly need the solution.

string Query = "UPDATE Members SET MemberID='" + MemberIDTextBox.Text + "'|| Name='" + NameTextBox.Text + "'|| Gender='" + GenderComboBox.Text + "'|| Address='" + AddressTextBox.Text + "'|| NID='" + NationalIDTextBox.Text + "'|| DOB='" + DOBTimePicker.Text + "'|| BloodGroup='" + BloodGroupTextBox.Text + "'|| Height='" + HeightTextBox.Text + "'|| Weight='" + WeightTextBox.Text + "'|| ChestSize='" + ChestSizeTextBox.Text + "'|| MusclesSize='" + MusclesSizeTextBox.Text + "'|| AbsPack='" + AbsPackTextBox.Text + "'|| Profession='" + ProfessionTextBox.Text + "'|| Contact='" + ContactTextBox.Text + "' WHERE MemberID='" + MemberIDTextBox.Text + "'";


SqlCommand Command = new SqlCommand(Query, Connection);
Command.ExecuteNonQuery();
Ran
  • 5,989
  • 1
  • 24
  • 26
pinGOL
  • 1
  • 4
  • What error message are you getting? – Bob Kaufman Jul 26 '16 at 18:05
  • I believe you want to use a data adapter as the binding source of your grid. Then in your update button you have a command like DataAdapter.SaveChanges(). I have not done this in a while but I remember there were plenty of examples on the net. – Joe C Jul 26 '16 at 18:05
  • 1
    What are the `||` symbols supposed to be? – Crowcoder Jul 26 '16 at 18:07
  • I think it will do as "OR". – pinGOL Jul 26 '16 at 18:08
  • 1
    See this previous SO answer [Paramterized Queries](http://stackoverflow.com/a/38575547/1260204) which has an example of an INSERT statement which you could convert to an UPDATE statement. It shows how to utilize a parameterized query with best practice hints. – Igor Jul 26 '16 at 18:09
  • I don't know exactly how to make it work as I want. I just want; after I change any of the TextBoxes value and click on Update button, instantly the table "Members" would updated. How should I write the code in Update button click event?? – pinGOL Jul 26 '16 at 18:11
  • SQL Injection Alert! Arggghhhh! http://www.c-sharpcorner.com/UploadFile/0926bc/sql-injection/ – Geoff James Jul 26 '16 at 18:12
  • You just have the update query all wrong. See codenoir's answer and my comment. – Crowcoder Jul 26 '16 at 18:13
  • @Crowcoder, yes sir. my code is all wrong but I want the right one. please help. – pinGOL Jul 26 '16 at 18:18

2 Answers2

2

Using a parameterized query:

SqlCommand cmd = new SqlCommand("update Members set Name=@Name, Gender=@Gender, Address=@Address, NID=@NID, DOB=@DOB, BloodGroup=@BloodGroup, Height=@Height, Weight=@Weight, ChestSize=@ChestSize, MusclesSizes=@MusclesSizes, AbsPack=@AbsPack, Profession=@Profession, Contact=@Contact" + " where MemberID=@MemberID", Connection);
cmd.Parameters.AddWithValue("@MemberID", MemberIDTextBox.Text);
cmd.Parameters.AddWithValue("@Name", NameTextBox.Text);
// ...
//Keep adding parameters
cmd.ExecuteNonQuery();
Henrique Forlani
  • 1,325
  • 9
  • 22
  • Its not working still. It says that "SqlException was unhandled" – pinGOL Jul 26 '16 at 18:51
  • If MemberID is your primary key, you can't update it. I've edited the code – Henrique Forlani Jul 26 '16 at 18:53
  • And btw, start trying with a few parameters until you have fixed the error: SqlCommand cmd = new SqlCommand("update Members set Name=@Name" + " where MemberID=@MemberID", Connection); – Henrique Forlani Jul 26 '16 at 18:56
  • Yes.. I did exactly like this but still its not working and says that "SqlException was unhandled" – pinGOL Jul 26 '16 at 18:58
  • did you convert your MemberID to int? are you using the correct types? – Henrique Forlani Jul 26 '16 at 19:03
  • oh , I forgot to ask, but how's your connection string? is it correct? – Henrique Forlani Jul 26 '16 at 19:07
  • Yes. MemberID type I used is int. As I said, I've already added some data into database using insert command and I retrieve the data from DataGridView in their individual TextBoxes when I click in a row of DataGridView. I just want now to change any of the TextBoxes value and Update. – pinGOL Jul 26 '16 at 19:07
  • Yes, I'm using the same correct ConnectionString that I used in insert command to insert data in the same form. – pinGOL Jul 26 '16 at 19:09
  • 2
    You need to look at the InnerException or Message property of the SqlException. – Xavier J Jul 26 '16 at 19:10
1

Solution: Change the double-pipe (||) characters to comma (,)

Like so: 

string Query = "UPDATE Members SET MemberID='" + MemberIDTextBox.Text + "',  Name='" + NameTextBox.Text + "',  Gender='" + GenderComboBox.Text + "',  Address='" + AddressTextBox.Text + "',  NID='" + NationalIDTextBox.Text + "',  DOB='" + DOBTimePicker.Text + "',  BloodGroup='" + BloodGroupTextBox.Text + "',  Height='" + HeightTextBox.Text + "',  Weight='" + WeightTextBox.Text + "',  ChestSize='" + ChestSizeTextBox.Text + "',  MusclesSize='" + MusclesSizeTextBox.Text + "',  AbsPack='" + AbsPackTextBox.Text + "',  Profession='" + ProfessionTextBox.Text + "',  Contact='" + ContactTextBox.Text + "' WHERE MemberID='" + MemberIDTextBox.Text + "'";

But you should really consider a parameterized query. You're just ASKING for a SQL Injection attack with your current implementation.

How do I use parameterized queries?

There's already a well-explained answer here, on StackOverflow, that gives good examples of how to use parameterized queries in your code.

What is SQL Injection?

It is where piece of SQL code is essentially exposed through simple string concatenation - mostly from user input (text fields etc.) as in your OP.

If a malicious "query" is input into said field, the potential injector could cause severe issues, and cause a lot of damage, access/edit parts of the database they're not meant to etc.

It is not only a pet hate for most of the world's programmers when other developers expose their code to SQL Injection, it's a real-world problem that can do (and has) destroyed businesses.

SQL Injection attacks are virtually "blind", however they can be very desctructive.

Some useful resources

Community
  • 1
  • 1
Xavier J
  • 4,326
  • 1
  • 14
  • 25
  • ... and if `MemberID` (or anything else) is numeric, don't put single quote around the value. And if it is an identity field you cannot update it. – Crowcoder Jul 26 '16 at 18:11
  • 2
    Which is another reason to use parameterized queries - you don't have to deal with what needs to be quoted or not. – Tim Jul 26 '16 at 18:15
  • Yes sir, I've replaced all double-pipe with comma. But still it gives me error. Its not working. :-( – pinGOL Jul 26 '16 at 18:22
  • It would have helped quite a bit if you posted the error message you're getting. – Xavier J Jul 26 '16 at 18:23
  • @pinGOL, you have to provide the error when you say you have an error. I assume it it because `MemberID` is a primary key and you cannot update it. Remove `MemberID` from the query. I suggest you update your question to show what you have now tried and what error you are getting. – Crowcoder Jul 26 '16 at 18:24
  • But also ... Forlani has posted a parameterized version of your SQL call. I'd try it. – Xavier J Jul 26 '16 at 18:24
  • @Crowcoder, it says that "SqlException was unhandled" I think the query is still not correct at all. – pinGOL Jul 26 '16 at 18:32
  • Looking at your so-called "query", my guess is that your "`MemberID`" column uses an integer-based column (`smallint` at a guess)? As @Crowcoder mentioned - numeric values don't require `'` around them, and you can't update identity fields. As per various comments and in this answer, using **Parameterized Queries** will undoubtedly solve all of life's problems for you :) – Geoff James Jul 26 '16 at 18:44
  • I've updated the code like @Forlani mentioned. But still its not working.. – pinGOL Jul 26 '16 at 18:55