Objects and instance variables in loops

Question

Is there anything wrong with doing this:

Public Class Form1
 Private Sub Form1_Load(ByVal sender As Object, ByVal e As System.EventArgs) Handles Me.Load
 For value As Integer = 0 To 1000000
 Dim p As New Person
 p.ID = value
 p.DoSomething()
 p = Nothing
 Next
 End Sub
 Public Class Person
 Dim _ID As Integer
 Public WriteOnly Property ID() As Integer
 Set(ByVal value As Integer)
 _ID = value
 End Set
 End Property
 Public Sub DoSomething()
 _ID = _ID + 1
 'Do more with ID
 End Sub
 End Class
End Class

As appose to this:

Public Class Form1
 Private Sub Form1_Load(ByVal sender As Object, ByVal e As System.EventArgs) Handles Me.Load
 For value As Integer = 0 To 1000000
 Person.DoSomething(value)
 Next
 End Sub
 Public Class Person
 Public Shared Sub DoSomething(ByVal id As Integer)
 id = id + 1
 'Do more with ID
 End Sub
 End Class
End Class

I have written some code similar to that in sample 1. I have read online that code 1 can cause problems when threading is involved, but I do not understand why? Are there any other problems with sample 1? Sample 1 meets my requirements better.

Answer 1

In sample1, as you call it, you have an instance field which you change. Therefore, the Person class you are using is mutable. Mutable classes are prominent to concurrency issues if a single instance is manipulated by multiple threads, unless explicit synchronization mechanism is properly involved.

In sample2, if you pass the value in the method, and you are not modifying the state of the Person class as you do your business logic, then the code is thread safe. No other thread can access the local variable id and modify it concurrently, since it is visible to the current thread only. So there is no need to apply a synchronization mechanism.

However, sample2 may not be appropriate enough for your concrete requirements. If you give more code clues on what happens in the method in question, then a more-accurate advice can be given.

Answer 2

The reason that the first example would be considered problematic when multiple threads are involved, is because in that example, a Person object maintains state (i.e. data) and it has methods that act upon that state. There's nothing wrong with designing classes that way, per se, and it doesn't mean that the class cannot be used by multi-threaded applications safely--it just makes multi-threading more difficult.

For instance, consider this example:

Public Class Form1
 Dim _p As New Person
 Private Sub DoWork(value As Integer)
 _p.Id = value
 _p.DoSomething()
 End Sub
 ' ... Code that calls DoWork on multiple threads ...
 Public Class Person
 Dim _id As Integer
 Public WriteOnly Property Id() As Integer
 Set(ByVal value As Integer)
 _id = value
 End Set
 End Property
 Public Sub DoSomething()
 _id = _id + 1
 'Do more with Id
 End Sub
 End Class
End Class

Let's say that in the above example, the DoWork method is called from two different threads simultaneously, and the order that the lines are processed is like this:

1. Thread A sets _p.Id = 1
2. Thread B sets _p.Id = 10
3. Thread A calls _p.DoSomething which increments _p.Id to 11
4. Thread B calls _p.DoSomething which increments _p.Id to 12

As you can see, that would certainly be undesired behavior. There are three ways to fix this problem. The first way is to use one of the available synchronization techniques to make sure that only one thread uses the object at a time, for instance:

Private Sub DoWork(value As Integer)
 SyncLock _p
 _p.Id = value
 _p.DoSomething()
 End SyncLock
End Sub

By synchronously locking on the object, if a second thread tries to call DoWork at the same time, the second thread's execution will be blocked (i.e. hung) until the first thread's call to DoWork is complete. The problem with this approach is that, at least to some extent, it defeats the purpose of multi-threading because only one thread can do that part of the work at a time.

The second way to fix this would be to have each thread create a new Person object so that they don't interfere with each other. For instance, instead of having the _p object declared as a class-level field, you could just create a new Person object inside the DoWork method, like this:

Private Sub DoWork(value As Integer)
 Dim p As New Person()
 p.Id = value
 p.DoSomething()
End Sub

The third way to fix this would be to use your second example. By fixing the Person class so that it is a stateless class, multiple threads can call the same object as often at the same time as necessary because the data is stored outside of the Person class. So, as long as each thread is instantiating a separate object to store the data before passing it into the Person object, all the threads can share the same Person object without needing to worry about collisions.

• w0051977: Thanks. +1. I assume that if I do not create any Threads myself i.e. using the Thread class, then there is no issues?
• Steven Doggart: It's not a matter of how you create your threads--it's a matter, rather, of how the threads use your Person class. If multiple threads share the same Person object, then you should have some concerns. If they each use their own independent Person object, then it's not a concern (unless the Person class contains shared members or shares dependencies between instances).
• w0051977: Thanks for the helpful comment. I assume that if I do not explicitly create any Threads then there is no issue to even think about? The reason I ask is because I know that ASP.NET uses MTA's (multi threaded apartments).
• Steven Doggart: Correct. If you are not explicitly making your code multi-threaded, none of it is an immediate concern.
• w0051977: Thanks I assume that the fact that ASP.NET uses MTA is insignificant. I know what MTA's are in theory but I do not understand the relationship between MTAs and threads (System.Thread). Thanks again.