3

I am new at programming and I am trying in vain to create a function that returns an auto increment id, however the code isn't working and I'm sure there's something missing even though I don't get what is it. Here the code, it was meant to create a function that everytime is called it creates an auto increment id, as if it is on SQL(a primary key):

class Program
{
    static void Main(string[] args)
    {
        string[] dataInsert = new string[10];

        for(int i = 0; i < dataInsert.Length; i++)
        {
            dataInsert[i] = Convert.ToString(generateId());
        }

        for (int i = 0; i < dataInsert.Length; i++)
        {
            Console.WriteLine(dataInsert[i]);
        }

        Console.ReadKey();
    }
    static int generateId()
    {
        int id = 1;
        return id += 1;
    }
}
maccettura
  • 10,514
  • 3
  • 28
  • 35
Ricardo
  • 87
  • 1
  • 2
  • 9
  • Is this id meant to be persisted across runs of the program? Or does it just need to start at 1 every time the program is ran? – maccettura Aug 01 '18 at 20:26
  • it is meant to start with 1 for the first time, and increment everry time the function is called – Ricardo Aug 01 '18 at 20:29
  • 4
    I understand that, but after you stop your program and start it again, should it start at 1 or from the last number? If you want it to start from the last number you need to persist the number somewhere (a database, a file, etc) – maccettura Aug 01 '18 at 20:31
  • should start from the last number, I got the files on txt to be inserted – Ricardo Aug 01 '18 at 20:41
  • 1
    What should happen if two instances of the program are running at one time? – mjwills Aug 01 '18 at 21:20

4 Answers4

12

You can make a static integer that remembers the previously returned id, then increment it as required using Interlocked.Increment():

using System.Threading;


class Program
{
    static int lastId = 0;

    static int generateId()
    {
        return Interlocked.Increment(ref lastId);
    }

    // Remainder of Program unchanged
}

Interlocked.Increment increments a specified variable and stores the result, as an atomic operation and returns the incremented value. It guarantees that duplicate IDs will not get returned when generateId() is called from different threads.

Note the following remark from the documentation:

This method handles an overflow condition by wrapping: if location = Int32.MaxValue, location + 1 = Int32.MinValue. No exception is thrown.

Thus you may want to add logic to generateId() to check for this and throw an exception.

For further reading about static fields and when to use them, see:

Update

In comments, the following question is asked: Why is Interlocked.Increment necessary here if we don't know that there are multiple threads (i.e. assuming this is a single-threaded app)?

By this point in time it should always be assumed that an app is multi-threaded. Thread-unsafe patterns should not be used if there is a simple, thread-safe alternative that can be used at no significant additional cost (in either development cost or runtime performance), as is the case here. And since OP self-describes as a Student at University of Madeira they should start out learning thread-safe patterns even when writing console apps so that they don't need to unlearn thread-unsafe techniques later.

dbc
  • 104,963
  • 20
  • 228
  • 340
  • Why is `Interlocked.Increment` necessary here if we don't know that there are multiple threads (i.e. assuming this is a single-threaded app)? – Kenneth K. Aug 01 '18 at 20:56
  • There is an additional cost. There's a lot of overhead to handle that interlock compared to a single increment. I have, however, run into a recent issue where I was trying to do some work with multiple threads and one of the libraries I was using wasn't thread-safe and kept causing random crashes. That was annoying! :-) It's important to know how to write thread-safe code, but I wouldn't say you always need to do it. Especially when it's your fun little app that you're completely in control of. If it's a library you plan to share, then sure. You should. – Jim Berg Aug 01 '18 at 21:45
  • *There's a **lot** of overhead to handle that interlock compared to a single increment.* Any evidence for that? [this answer](https://stackoverflow.com/q/1034070) and [this article](https://learn.microsoft.com/en-us/windows/desktop/DxTechArts/lockless-programming) indicate that the performance penalty is 36-90 cycles which is minuscule for something like an ID generator, and only becomes significant when used in some tight loop. But I was more interested in *development cost* rather than *performance cost*, and Interlocked.Increment is just as easy as an increment operator here. – dbc Aug 01 '18 at 21:52
  • Interlocked.Increment(ref id) takes more time to type than Id++. That's way more development time than the 4-5 times longer it will take to execute;-) They should have abbreviated it. IntLk.Inc maybe? :) – Jim Berg Aug 02 '18 at 04:31
8

Change

static int generateId()
{
    int id = 1;   //This resets the value to 1 every time you enter.
    return id+= 1;
}

To:

private static int id = 1;
static int generateId()
{
    return id++;
}

Your generateId will always return 2 since you're passing out a local value that you initialize to 1 each time it's entered. With my example, Id will be set to 1 when the app is run and then increments. It will always start at 1, though.

Adding explanation of local variables, field variables, and static variables.

When you define a variable like you did, a local variable, it only exists within the scope of the method and will get reinitialized each time the method is called.

You can also define field variables that will exist as long as an instance of an object exists. For example:

public class AutoIncrment
{
    private int id = 1;
    public int GenerateId()
    {
       return id++;
    }
}

You can use it like this:

var idState = new AutoIncrement();

console.WriteLine(idState.GenerateId()); // Outputs 1
console.WriteLine(idState.GenerateId()); // Outputs 2 
console.WriteLine(idState.GenerateId()); // Outputs 3 
console.WriteLine(idState.GenerateId()); // Outputs 4 .. And so on

var idState2 = new AutoIncrement();  // Create a new instance and it starts over

console.WriteLine(idState2.GenerateId()); // Outputs 1
console.WriteLine(idState2.GenerateId()); // Outputs 2 .. And so on
// Go back to idState object and it will keep going from where you last left off
console.WriteLine(idState.GenerateId()); // Outputs 5 

Then there are static variables like in my answer at the top. They exist as part of the class itself. Since the class always exists while the program is running, the variable exists in memory, too.

Jim Berg
  • 609
  • 4
  • 7
0

Simply need to make your int static.

Also declare in the class space. Not in the function.

ErnieDingo
  • 444
  • 5
  • 11
0

your code always retun 2 in generateid() method :

       static int id =1;
       for(int i=0;i<dataInsert.Length;i++)
        {
            dataInsert[i] = Convert.ToString(generateId());
        }

        static int generateId()
        {

          return id+= 1;
        }
Mohammad
  • 1,549
  • 1
  • 15
  • 27