-7

I'm trying to return the InvoiceID from the database but it keeps returning -1.

I'm trying to select the invoice ID From tblInvoice to display it on a form and also use it to insert it into a second table that has a one to many relationship with tblInvoice.

try
{
    conn = new SqlConnection(conString);
    conn.Open();
    string select = "SELECT TOP 1 FId FROM tblFaktuur ORDER BY FId DESC";
    SqlCommand cmd1 = new SqlCommand(select, conn);
    i = (int)cmd1.ExecuteNonQuery();
}
catch (Exception e)
{
    System.Windows.Forms.MessageBox.Show("ReadInvoiceNumber()" + e.Message);
}

return i;

When Running the query SELECT TOP 1 FId FROM tblFaktuur ORDER BY FId DESC in SQL Server it returns the value of 6 which is the last Invoice ID.

GSerg
  • 76,472
  • 17
  • 159
  • 346
  • 10
    Think about that code. How do you expect a method called Execute**Non**Query to handle a Query? Change that to `ExecuteScalar`. – Camilo Terevinto Jun 13 '19 at 16:22
  • 1
    Possible duplicate of [ExecuteNonQuery for SELECT sql statement returning no rows](https://stackoverflow.com/questions/4269548/executenonquery-for-select-sql-statement-returning-no-rows) – nofinator Jun 13 '19 at 16:23
  • If you run query on SQL Server Management Studio do you get results? SSMS error messages are much better than the c# errors. I always recommend testing all queries on SSMS before writing c# code. – jdweng Jun 13 '19 at 16:26
  • 3
    On a side note, this kind of code is almost never correct to have. Almost always people perform that kind of query to either calculate the next ID based on the last used ID or to learn what ID was assigned to a document they just created. In both cases it is very wrong to use that query, as it will only correctly work in a single user environment. – GSerg Jun 13 '19 at 16:27
  • @jdweng There are no errors. – GSerg Jun 13 '19 at 16:28
  • 2
    And on top of everything else, SQLConnection should be disposed, same as SqlCommand. and the catch clause is using a MessageBox which means it's not really decoupled from the UI... – Zohar Peled Jun 13 '19 at 16:32
  • int bestPracticeStandards = (int)Common.StandardType.None; – jPhizzle Jun 13 '19 at 16:36
  • But do you get any data returned? c# will not show all the errors that SSMS will show. You may think there are no errors, but there still can be errors. – jdweng Jun 13 '19 at 16:39
  • Thanks a lot for everyone's help I am still learning everything as a second year student. @GSerg if you have any suggestions on a better method please if you are able send me a link or an example. – Reinhard Engelbrecht Jun 13 '19 at 16:40
  • @ReinhardEngelbrecht A better method to do exactly what? – GSerg Jun 13 '19 at 16:40
  • SSMS shows the correct value in this case it is 6 and then c# returns -1 i changed it to ascending in that case it was suppose to return 4 but it also returned -1.@jdweng – Reinhard Engelbrecht Jun 13 '19 at 16:41
  • Look at the very first comment... – Sean Lange Jun 13 '19 at 16:42
  • The main goal of the code is to get the Invoice id and be able to display it and also increase that number by one to be able to give it to the table which has a relationship with the invoice table – Reinhard Engelbrecht Jun 13 '19 at 16:42
  • 1
    What you are describing as the point of this code is the problem that @GSerg was mentioning. You have created a race condition to handle what should be done using the identity property. – Sean Lange Jun 13 '19 at 16:43

1 Answers1

0

Think about that code. How do you expect a method called ExecuteNonQuery to handle a Query? Change that to ExecuteScalar. This work Perfectly Thanks to @Camilo Terevinto, i will be doing research to improve my skills atleast i know what is wrong.

  • Additionally if you don't properly close and dispose of your connection and command objects you are going to cripple your server by consuming all of the available connections. Wrap both of those objects (and any objects that inherit IDisposable) in a USING statement. – Sean Lange Jun 13 '19 at 16:48