tags:

views:

476

answers:

10

I would like to give a class a unique ID every time a new one is instantiated. For example with a class named Foo i would like to be able to do the following

dim a as New Foo()
dim b as New Foo()

and a would get a unique id and b would get a unique ID. The ids only have to be unique over run time so i would just like to use an integer. I have found a way to do this BUT (and heres the caveat) I do NOT want to be able to change the ID from anywhere. My current idea for a way to implement this is the following:

Public Class test
    Private Shared ReadOnly _nextId As Integer
    Private ReadOnly _id As Integer
    Public Sub New()
        _nextId = _nextId + 1
        _id = _nextId
    End Sub
End Class

However this will not compile because it throws an error on _nextId = _nextId + 1 I don't see why this would be an error (because _Id is also readonly you're supposed to be able to change a read only variable in the constructor.) I think this has something to do with it being shared also. Any solution (hopefully not kludgy hehe) or an explanation of why this won't work will be accepted. The important part is i want both of the variables (or if there is a way to only have one that would even be better but i don't think that is possible) to be immutable after the object is initialized. Thanks!

A: 

It's likely throwing an error because you're never initializing _nextId to anything. It needs to have an initial value before you can safely add 1 to it.

DannySmurf
A: 

It throws an error because _nextId is ReadOnly. Remove that.

Edit: As you say, ReadOnly variables can be changed in a constructor, but not if they are Shared. Those can only be changed in shared constructors. Example:

Shared Sub New()
   _nextId = 0
End Sub
Magnus Akselvoll
Did you read the question? He wants _nextId to be readonly.
DannySmurf
I didn't know you could comment on answers so ignore my reply below this, this makes sense. I'm leaving this open for a bit to see if anyone else has any interesting ideas but than i will accept this answer. Thank you i didn't know a shared read only could only be modified in a shared constructor.
Nicholas
A: 

Magnus, you can change a readonly variable in the Constructor of the class that owns it, so that isn't why it is NOT compiling (and not throwing an error)

Danny, it isn't throwing an error it isn't compiling, also initializing it to zero does not seem to help either.

Nicholas
+5  A: 

This design is vulnerable to multithreading issues. I'd strongly suggest using Guids for your IDs (Guid.NewGuid()). If you absolutely must use ints, check out the Interlocked class. You can wrap all incrementing and Id logic up in a base class so that you're only accessing the ID generator in one location.

Will
+1  A: 

ReadOnly variables must be initialized during object construction, and then cannot be updated afterwards. This won't compile because you can't increment _nextId for that reason. (Shared ReadOnly variables can only be assigned in Shared constructors.)

As such, if you remove the ReadOnly modifier on the definition of _nextId, you should be ok.

Remi Despres-Smyth
A: 

I'd do it like this.

Public MustInherit Class Unique
    Private _UID As Guid = Guid.NewGuid()
    Public ReadOnly Property UID() As Guid
        Get
            Return _UID
        End Get
    End Property
End Class
Joel Coehoorn
A: 

The shared integer shouldn't be read-only. A field marked readonly can only ever be assigned once and must be assigned before the constructor exits.

As the shared field is private, there is no danger that the field will be changed by anything external anyway.

Sarcastic
A: 

Consider the following code:

Public Class Foo 
    Private ReadOnly _fooId As FooId 

    Public Sub New() 
        _fooId = New FooId() 
    End Sub 

    Public ReadOnly Property Id() As Integer 
        Get 
            Return _fooId.Id 
        End Get 
    End Property 
End Class 

Public NotInheritable Class FooId 
    Private Shared _nextId As Integer 
    Private ReadOnly _id As Integer 

    Shared Sub New() 
        _nextId = 0 
    End Sub 

    Public Sub New() 
        SyncLock GetType(FooId) 
            _id = System.Math.Max(System.Threading.Interlocked.Increment(_nextId),_nextId - 1) 
        End SyncLock 
    End Sub 

    Public ReadOnly Property Id() As Integer 
        Get 
            Return _id 
        End Get 
    End Property 
End Class

Instead of storing an int inside Foo, you store an object of type FooId. This way you have full control over what can and cannot be done to the id.

To protect our FooId against manipulation, it cannot be inherited, and has no methods except the constructor and a getter for the int. Furthermore, the variable _nextId is private to FooId and cannot be changed from the outside. Finally the SyncLock inside the constructor of FooId makes sure that it is never executed in parallell, guaranteeing that all IDs inside a process are unique (until you hit MaxInt :)).

Magnus Akselvoll
This does what i want, never thought about doing it this way kudos, so I'm going to accept it as an answer but i strongly recommend against doing it this way. The other answer was right a GUID is definitely the way to go.
Nicholas
Locking on the type object is a bad idea. Anybody can get the type object and lock on it. Lock objects should be kept private within a class instance. Also, locking and then using Interlocked.Increment is redundant. the Interlocked class methods are atomic and don't require locking.
Will
A: 

You said that "this will not compile because it throws an error" but never said what that error is.

A shared variable is static, so there is only a single copy of it in memory that is accessible to all instances. You can only modify a static readonly (Shared ReadOnly) from a static (Shared) constructor (New()) so you probably want something like this:

Public Class test
   Private Shared ReadOnly _nextId As Integer
   Private ReadOnly _id As Integer

   Public Shared Sub New()
      _nextId = _nextId + 1
   End Sub

   Public Sub New()
      _id = _nextId
   End Sub
End Class

(I think that's the right syntax in VB.) In C# it would look like this:

public class Test
{
   private static readonly int _nextId;
   private readonly int _id;

   static Test()
   {
      _nextId++;
   }

   public Test()
   {
      _id = _nextId;
   }

}

The only problem here is that the static constructor is only going to be called once, so _nextId is only going to be incremented one time. Since it is a static readonly variable you will only be able to initialize it the static constructor, so your new instances aren't going to be getting an incremented _id field like you want.

What is the problem you are trying to solve with this scenario? Do these unique IDs have to be integer values? If not, you could use a Guid and in your contructor call Guid.

Scott Dorman
A: 

I posted a similar question that focused on the multithreading issues of setting a unique instance id. You can review it for details.

Anthony Mastrean