0

I have created a daily sales recording system for a grocery shop where until 1000 records it works perfectly. After 1000th record its not auto-incrementing but I can manually change the primary key value and insert the data as required.

This is the code I have used for auto incrementing:

void oil_ID_auto()
{
    try
    {
        SqlCommand selectCommand = new SqlCommand("Select OilId from Oiltbl", conn);
        SqlDataReader reader = selectCommand.ExecuteReader();
        bool rowFound = reader.HasRows;
        string OilId = null;
        if (rowFound)
        {
            while (reader.Read())
                OilId = reader[0].ToString();//003
            string customerIDString = OilId.Substring(1);
            int UserID = Int32.Parse(customerIDString);
            int ProductIdInt = 0;
            if (UserID >= 0 && UserID < 9)
            {
                ProductIdInt = UserID + 1;
                txtoid.Text = "O00" + ProductIdInt;
            }
            else if (UserID >= 9 && UserID < 99)
            {
                ProductIdInt = UserID + 1;
                txtoid.Text = "O0" + ProductIdInt;
            }
            else if (UserID >= 99)
            {
                ProductIdInt = UserID + 1;
                txtoid.Text = "O" + ProductIdInt;
            }
        }
        else
            txtoid.Text = "O001";
        reader.Close();
    }
    catch (Exception ex)
    {
        MessageBox.Show("Error on oil ID generating" + ex.Message, "MAIN Form", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }
}

I expect the output as O1001,O1002 etc...but actually it remains O1000 even after O1000 record is inserted

AbdelAziz AbdelLatef
  • 3,650
  • 6
  • 24
  • 52
  • What is the definition of `Oiltbl`? How is it populated? You really haven't given us anything to go on. – Ian Kemp Sep 07 '19 at 16:43
  • 5
    Your `while` loop to read all OilIds or the last one looks wrong and rely on how the database is returning the rows, specially when you don't specify the order the rows are returned with an `ORDER BY` clause in the SQL query. You might want to check the query to `SELECT OilId from Oiltbl ORDER BY OildId DESC LIMIT 1` or something similar. Then you can get rid of the `while` loop since you will get only one row (at most) anyway. – Progman Sep 07 '19 at 16:43
  • You might want to look at https://stackoverflow.com/questions/4325267/c-sharp-convert-int-to-string-with-padding-zeros to fill up an integer value with zeros at the beginning. – Progman Sep 07 '19 at 16:46
  • 2
    This whole scheme is a bad idea that sets up a HUGE race condition for two users to create the same ID value. You want an **IDENTITY COLUMN**, and only show the new ID to the user after it's created. – Joel Coehoorn Sep 07 '19 at 17:38
  • Joel Coehoorn how can i do that?? – Mohamed Ifham Sep 08 '19 at 08:52

1 Answers1

3

I suggest a simpler approach

using (var conn = new SqlConnection(connectionString))
using (var command = new SqlCommand("SELECT MAX(OilId) FROM Oiltbl", conn)) {
    conn.Open();
    object result = command.ExecuteScalar();
    if (result == null) {
        txtoid.Text = "O001";
    } else {
        int UserId = (int)result; // Assuming that OilId is an int column.
        int ProductIdInt = UserId + 1;
        txtoid.Text = $"O{ProductIdInt:d3}";
    }
}

Use SELECT MAX(OilId) to return only one record containing the largest id.

ExecuteScalar returns the first column of the first record, or null if no such row was found.

C#'s string interpolation (C# reference) together with string composite formatting automatically creates the desired result. The format d3 automatically inserts leading zeroes if the number has less than 3 digits.


If the OilId column is a text column, you will have to treat it as string and parse the string

int UserId = Int32.Parse((string)result.Substring(1));

Note that if you are storing the id as text, the maximum id will be the last id in lexical order!

O100
O1000
O101
... 
O998
O999

So, the id O1000 appears between O100 and O101!

MAX(OilId) returns O999! Therefore your code will repeatedly try to insert O1000.

There are two approaches to solve this problem

  1. Use an int column for the id, and prepend the "O" only when formatting the id in the UI. In addition to solving your problem, this allows you to use an identity column which automatically generates and increments the ids. This is by far the easiest and most robust solution.

  2. Make all the ids long enough to account for future bigger numbers: O000001. Replace any existing shorter ids with the longer format. As @Joel Coehoorn says in his comment, you are getting troubles with this solution, if two users are trying to insert a new record at about the same time.

Olivier Jacot-Descombes
  • 104,806
  • 13
  • 138
  • 188
  • Already 1000 records of data in the text format in the database such that (O001,O002 etc..),If i change the id for interger now will it work?? – Mohamed Ifham Sep 07 '19 at 18:24
  • You must first remove the leading "O". `UPDATE theTable SET id = SUBSTRING(id, 2, LEN(id)) WHERE id LIKE 'O%'`. the TSQL substring function is 1-based, not 0 based as in C#. See also [Change type of a column with numbers from varchar to int](https://stackoverflow.com/questions/577712/change-type-of-a-column-with-numbers-from-varchar-to-int). – Olivier Jacot-Descombes Sep 07 '19 at 18:29
  • How can i use an int column for id..what are the things i have To change in the above code – Mohamed Ifham Sep 08 '19 at 08:54
  • With an identity key, the above code is superfluous, as the key will be generated automatically when you insert a record. See [Execute Insert command and return inserted Id in Sql](https://stackoverflow.com/questions/18373461/execute-insert-command-and-return-inserted-id-in-sql). – Olivier Jacot-Descombes Sep 08 '19 at 12:31