Are there known issues with shared functions?

[davearia]

Hi All,

 

Hope you are well, once again it's been a while xsince my last visit. Still busy doing house things.

 

Came across an issue a week or so back at work regarding the use of shared functions. Basically the code I was looking at had a customer structure that was declared as shared. The bottom line of this was every time our web site was busy on this page quite often a customer would update their details but in fact as these structures would quite often have the wrong Sales ID so in fact they updated somebody elses instead.

 

Took a while to find the problem as this never anywhere else apart from the live system as it needed more than one person to cause this problem. Anyway I put some database logging code in and discovered what the problem was.

 

Since the code has gone out this error has stopped.

 

Now I am looking at another problem that I suspect could be caused by the same thing. There is a function in our site that calculates VAT. In our error logs with sometimes get quite a few errors to this effect: Count cannot be less than zero Parameter name count. When I was looking through this class I noticed there was aroung 10 functions. None are shared except the function that errors.

 

I was wondering if there is known issues with shared functions in web development and it is considered generally bad practice to use them?

 

Thank, Dave.

[MrPaul]

Shared is fine

 

I was wondering if there is known issues with shared functions in web development and it is considered generally bad practice to use them?

 

No on both counts - the framework is full of shared/static methods which work just fine in web environments, so the problem is likely due to some logical error in the function itself. We might be able to provide some specific help/insight if you could post the code in question.

[HardCode]

I have a very similar question, so to avoid a duplicate thread, perhaps I can join in on this discussion of Shared, and davearia and I can both benefit from a response. I was wondering how a Public Shared function in a "helper" class, which validates a string for Not String.IsNullOrEmpty and also validated length, would be affected by multiple users calling this function at the same time. The answer will probably be the same for davearia's "calculates VAT" question above?

 

EDIT: MrPaul got in an answer before I finished the post, so I assume that I'd not have a problem with Shared itself for my example?

[PlausiblyDamp]

If the functions in question also manipulate some form of shared storage (global variables, class level shared variables etc.) then these shared resources are not going to be thread safe and therefore will cause problems when used in this manner.

 

Pure shared functions that don't modify any shared state though are perfectly safe to use and as MrPaul points out are used throughout the framework itself without any problems.

[MrPaul]

Shared functions and threads

 

I think you might be getting shared/static members confused with threading issues. Instance methods (and properties) are the same as shared methods except that an additional hidden parameter is passed in, which is a pointer to the object the method is being called on, so this:

 

i = someText.IndexOf("test")

 

actually does something like this behind the scenes:

 

i = String.IndexOf(someText, "test")

 

So, the only difference between shared and instance members is that instance members have an additional Me pointer (or this in C#) which is the instance the method is being invoked on. Otherwise there is no difference.

 

The upshot of this is that if you're having problems with multiple threads/users/whatever accessing a shared method at the same time, the problem is down to the way it is written, not the fact that it is shared. For example, the following shared method will cause no problems:

 

Public Shared Function ValidateString(ByVal someString As String) As Boolean
   If String.IsNullOrEmpty(someString) Then Return False
   If someString.Length < 5 Then Return False
   If someString.Length > 30 Then Return False
   If someString.IndexOfAny(New Char() {" ", "!", "$", "%"}) > -1 Then Return False
   Return True
End Function

 

However, a shared method which accesses a shared field might not be so safe:

 

Public Shared Function DoSomething(ByVal someText As String) As String
   MyClass.SharedField = someText

   'Do something with SharedField

   Return MyClass.SharedField
End Sub

 

Because here, another thread may enter the method and set MyClass.SharedField to a new value while the other thread is still using it. The proper way to do this would be something like:

 

Public Shared Function DoSomething(ByVal someText As String) As String
   SyncLock MyClass.SharedField
       MyClass.SharedField = someText

       'Do something with SharedField

       Return MyClass.SharedField
   End SyncLock
End Sub

 

Note that this sort of problem could also occur in an instance (non-shared) function.

 

Good luck :cool:

[davearia]

Hi,

 

Firstly thanks for your replies, given me something to think about.

 

I think that shared resources are a good idea using the shared modifier or a singleton class. Just with the example I fixed the other week that I mentioned was a good example of using shared modifier for a customer structure can't be a good use of this type of coding.

 

With the function I am trying to fix here is the jist of what is being coded:

Public Shared Function CalculateVAT(ByVal price As Decimal, ByVal vatrate As Decimal) As Decimal
           Dim d As Decimal = price * (vatrate / 100)
           Dim s As String = d.ToString()
           If s.Contains(".") Then
               s = s.Remove(s.LastIndexOf(".") + 3, (s.Length - (s.LastIndexOf(".") + 3)))
           End If
           Return price + Decimal.Parse(s)
       End Function

 

There is some risk using shared modifiers here isn't there?

 

Cheers, Dave.

[PlausiblyDamp]

AS there is nothing in the CalculateVAT function that relies on state information held within the class containing it there should be no problems making it a shared function. In fact making it shared better states the use / intent of the function anyway.

 

Just as an aside could you not use something smiilar to

Public Shared Function CalculateVAT(ByVal price As Decimal, ByVal vatrate As Decimal) As Decimal
       Dim d As Decimal = price * (vatrate / 100)
       d = (Decimal.Truncate(d * 100)) / 100

       Return price + d
End Function

instead and remove the conversion to / from a string when calculating the value?

Edited May 29, 2007 by PlausiblyDamp

[MrPaul]

Unnecessary string conversion

 

Indeed, the exception you were getting was caused by the call to s.Remove(...), which would have been indicated in a stack trace. As PlausiblyDamp demonstrates, there is no need to convert the number to and from a string.

 

Good luck :cool:

Edited May 29, 2007 by MrPaul

[HardCode]

Re: Unnecessary string conversion

 

Excellent explanations. It has helped me, also.

[davearia]

Many thanks for your experts advice again, it's really appreciated.

 

Ta, Dave.