-2

I am trying to update a SQL table from my C# backend, but it never successfully executes; mainServiceButton is a pre-existing value in the linkName column. Here is what I have so far:

conn.Open();
       string qry = "UPDATE clickStream SET clickCount = (clickCount + 1) WHERE linkName = mainServiceButton";
       SqlCommand cmd = new SqlCommand(qry, conn);
       try
       {
           cmd.ExecuteScalar();
       }
       catch
       {
           MessageBox.Show("not executed");
       }
        conn.Close();

This is how the table was created:

CREATE TABLE clickStream(
click_ID int identity(1,1),
linkName nvarchar(50) not null,
clickCount int,
PRIMARY KEY(click_ID));

The desired result is to increase the clickCount by 1 every time a link(linkName) is clicked on. Any Suggestions?

Douglas Tober
  • 300
  • 4
  • 15
  • 3
    I am at a loss for how the title of your question has anything to do with the question itself. – JohnFx May 06 '13 at 01:56
  • 4
    BTW: You want ExecuteNonQuery, not ExecuteScalar Are you getting an error or something? Your question doesn't have nearly enough info to answer. – JohnFx May 06 '13 at 01:57
  • 2
    "it never successfully executes"? Care to elaborate? – spender May 06 '13 at 01:57
  • My bad with the name...I started out with a different question and forgot to change the name... – Douglas Tober May 06 '13 at 02:09
  • 1
    I continuously get the MessageBox saying"not executed" because the cmd.executeScalar() was not working. I will try ExecuteNonQuery and update this post. Thanks for the help so far! – Douglas Tober May 06 '13 at 02:15
  • Don't swallow the exception. Use `throw` instead, at least until you have something useful to do in the catch, like error logging. – mbeckish May 06 '13 at 02:16
  • 2
    You also probably intended `linkName = mainServiceButton` to be more like `linkName = '" + mainServiceButton + "'"`... either was its not a good way to do it. Use parameters. – MikeSmithDev May 06 '13 at 02:16
  • 2
    Please provide the exception text – Andrew Savinykh May 06 '13 at 02:20
  • 1
    I hard coded the mainServiceButton value in for testing purposes, this will be replaced with a variable in the future and would look something like this: `linkName = " + var;` I am very new to C# and SQL can you provide an example of Using parameters Mike? – Douglas Tober May 06 '13 at 02:22
  • 1
    @user2350672 J0e3gan did. It helps protect against SQL injection. And use the @ sign when replying to people... you'll see their name popup when you do. Click on it. It will fill in their username and notify them of your comment. Otherwise they'll never know you asked them a question. – MikeSmithDev May 06 '13 at 02:51

2 Answers2

2

MessageBox.Show("not executed"); is not going to help you much except to obscure the details of the error: you need to instead output the details of the caught exception to understand what happened.

Addressing this and other suggestions made in comments...

  • mainServiceButton nakedly inline in the SQL text not possibly being what you want
  • a SqlParameter being warranted to accept a value for the WHERE sanely
  • ExecuteNonQuery() instead of ExecuteScalar() being the right call

..., see what sort of mileage you get with this instead:

conn.Open();
string qry = "UPDATE clickStream SET clickCount = (clickCount + 1) WHERE linkName = @linkName";
SqlCommand cmd = new SqlCommand(qry, conn);
// Use a SqlParameter to correct an error in the posted code and do so safely.
cmd.Parameters.Add(new SqlParameter("@linkName", "mainServiceButton"));
try
{
    cmd.ExecuteNonQuery(); // not ExecuteScalar()
}
catch (SqlException sex)
{
    // Output the exception message and stack trace.
    MessageBox.Show(sex.ToString());
}
conn.Close();
J0e3gan
  • 8,740
  • 10
  • 53
  • 80
  • Thank you so much! Everything is working the way I would like it to! I appreciate your patience and help! :) – Douglas Tober May 06 '13 at 13:00
  • 1
    @user2350672: Absolutely. When you find that an answer resolves an issue you post, be sure to mark the answer accepted to indicate as much. – J0e3gan May 06 '13 at 13:05
-1

Try the below, not tested so you may need to fix minor bugs:

conn.Open();
string qry = "UPDATE clickStream SET clickCount = (clickCount + 1) WHERE linkName = 'mainServiceButton';SELECT @@ROWCOUNT;";
SqlCommand cmd = new SqlCommand(qry, conn);
try
{
    int rowsAffected = (int)cmd.ExecuteScalar();
    if (rowsAffected != 1)
        throw new ApplicationException("Rows affected should be 1, " + rowsAffected + " were affected.");
}
catch (Exception ex)
{
    MessageBox.Show("Not executed successfully, exception: " + ex.ToString());
}
conn.Close();
David Cummins
  • 972
  • 6
  • 15
  • Care to comment on the downvote? It does what I believe the OP wants, plus has better error handling. – David Cummins May 06 '13 at 03:07
  • I didn't downvote, but I fail to see how it has better error handling. It also doesn't use parameters, which is what he needs. And a `using` statement would be better... (or really a new way to interface with his db :/). And why the execute scalar to update. – MikeSmithDev May 06 '13 at 03:12
  • My approach was to help him address his direct problem (the table not being updated). It will tell him what the exception is (if there is one) or that no rows are being updated due to an incorrect condition on his update. I've seen much worse data access code on SO, it'll do for now. – David Cummins May 06 '13 at 03:18
  • Helping him solve his problem while leaving him open to SQL injection is not helpful at all, really. He's already catching all exceptions and could have found out why it is failing by looking at it, as we already know there is an exception. – MikeSmithDev May 06 '13 at 03:19
  • My version didn't leave him open to SQL injection - the SQL is a constant string. But good point that I should have inferred what he was trying to do. As for "could have", well he didn't. I try to offer help at the level that I believe the poster is at. – David Cummins May 06 '13 at 03:31