2.1 Validation Rules - must they be Shared?

JoeFallon1 posted on Monday, August 28, 2006

Using code like this works as far as loading the rule goes. But is it mandatory to Share functions like this?

Note that the Function is not Shared. Must it be? Is there a reason Rocky does not check the IsStatic property when loading the rules? I would think that an Exception should be thrown if a non-Shared rule was added to the per Type Business rules.

 Protected Overrides Sub AddBusinessRules()
        ValidationRules.AddRule(AddressOf NoDuplicates, "Id")            

End Sub

Private Function NoDuplicates(ByVal target As Object, ByVal e As Validation.RuleArgs) As Boolean

      If 1 = 2 Then
        e.Description = "Id must be unique"
        Return False
      Else
        Return True
      End If

End Function

Joe

 

ajj3085 replied on Monday, August 28, 2006

If you want shared rules yes, NoDuplicates must be Shared (static in C#).  You'd probably want it shared anyway, so that you are thread safe (as long as you don't use any other Shared fields).

Of course if you need instance rulse for some reason, override AddInstanceBusinessRules and keep the method non-shared.

Andy

JoeFallon1 replied on Monday, August 28, 2006

I agree that I do want it to be Shared  especially since it is loaded from within AddBusinessRules. I just thought there would be some sort of exception if I added an instance rule to the wrong list.

Deeper in the code, Rocky combines both lists (Shared and Instance) if rules for both exist. So I guess it really doesn't matter if it is Shared or not - it matters which method loads the rule. AddBusinessRules will still only add the rule once per type. AddInstanceBusinessRules should add it once per instance.

Comments?

ajj3085 replied on Monday, August 28, 2006

There won't be any exceptions, but you may have 'weird' results if you're looking at the instance instead of the target parameter, which can be different.  I had this happen when I moved to the 2.1 beta refresh.. I had two objects in a  list, and the rule was looking at the 'first' object's properties even though it was the second object which was supposed to be running rule's checks..

Because of this I would now always recommend your rule methods are shared / static, even if they are implementing instance rules... that way you avoid any weird bugs like I've experienced.

xal replied on Monday, August 28, 2006

It's not mandatory that the rule methods are static...
The thing to keep in mind is that if you add shared rules, you must use the target in order to validate it.
This is obvious if the method is shared, but not if it's an instance method.

I questioned Rocky about the same subject not long ago... His reason for allowing instance methods was that he didn't want to restrict the way rules work or the programmer's creativeness :).
He had a "validator object" in his mind, of which you could have an instance as a singleton and use that to validate different objects.

Maybe a good way out would be disallow instance methods is the method is owned by the same type as the object being validated.... although that would add some overhead.

Anyway, I don't think it's bad to allow instance rule methods, but it should be well clarified that those methods _should_ be static if they are inside the bo.


One thing I usually do when I have many rules inside my bo is create a class named Rules inside my bo that has all the static methods for validation. That allows me to have them all handy, and I can also access my bo's private members.



Andrés

JoeFallon1 replied on Monday, August 28, 2006

That is good advice.

I will be sure to have all my Rule methods declared as Shared.

Thanks.

Joe

 

ajj3085 replied on Tuesday, August 29, 2006

xal:

It's not mandatory that the rule methods are static...

Right.. it was just my advise that rule methods are static.  If you always make them static, you MUST use the target parameter, and you won't make silly mistakes like I did. 

xal replied on Tuesday, August 29, 2006

Well, it's definitely something more than one will suffer.
I know I did all my rules in my classes and accessed private fields... and it will cause grief to anyone updating to 2.1 because they'll see that their rules won't work. It should be noted very very clearly in the release.

Andrés

skagen00 replied on Tuesday, August 29, 2006

"I know I did all my rules in my classes and accessed private fields... and it will cause grief to anyone updating to 2.1 because they'll see that their rules won't work."

Andres, could you clarify what you're saying? Where is the problem here? Are you saying that if you have a shared/static rule handler that you can't access private fields? That isn't the case now is it, as long as the handler is within the class definition?

(I just want to make sure I'm not missing something here!)

 

 

RockfordLhotka replied on Tuesday, August 29, 2006

If you have rule methods that are instance methods of your business class, and those methods directly access instance fields in the object, then when you move from 2.0 to 2.1 you'll need to rename AddBusinessRules() to AddInstanceBusinessRules() and inside that method rename calls to AddRule() to AddInstanceRule() for your application to continue to work properly.

Alternately, you can change your rule methods to be static/Shared, which means they can no longer use instance fields directly, but rather must rely on the target parameter to get the values.

I am looking into throwing an exception if you call AddRule() where the rule method is a non-static method of the business object type. That is the most common area of concern, and I think Xal is right - throwing an exception in this case will be the biggest possible help for people moving from 2.0 to 2.1, because it is really the only place you can get in serious trouble.

xal replied on Tuesday, August 29, 2006

skagen00:

It means that I used to do this:

Public Class MyBO
    Private mField As String
   
    Private Function MyRule(target As Object, e As Csla.Validation.RuleArgs) As Boolean
          e.Description = "Field cannot be empty"
          Return Not mField.Equals(String.Empty)
    End Function


End Class

And now it should be:

    Private Function MyRule(target As Object, e As Csla.Validation.RuleArgs) As Boolean
          e.Description = "Field cannot be empty"
          Return Not DirectCast(target, MyBO).mField.Equals(String.Empty)
    End Function


The last one will work fine whether it's an instance or a static method.
The problem with the instance method is that it will keep an instance of the object alive throughout the lifespan of your application, and that could be potentialy bad if your object is very big, but it's not a terrible situation.

Andrés

RockfordLhotka replied on Tuesday, August 29, 2006

And now it should be:

    Private Function MyRule(target As Object, e As Csla.Validation.RuleArgs) As Boolean
          e.Description = "Field cannot be empty"
          Return Not DirectCast(target, MyBO).mField.Equals(String.Empty)
    End Function

xal replied on Tuesday, August 29, 2006

Well, here's an even better example:

Public Class MyBO
    Inherits Csla.BusinessBase(Of MyBO)

    Protected Overrides Sub AddBusinessRules()
       MyBase.AddBusinessRules()
       ValidationRules.AddRule(Of MyBO, RuleArgs)( _
            AddressOf MyRule, New RuleArgs("Field"))
    End Sub

    Private Shared Function MyRule(target As MyBO, e As Csla.Validation.RuleArgs) As Boolean
          e.Description = "Field cannot be empty"
          Return Not target.mField.Equals(String.Empty)
    End Function
   
    Private mField As String
    Public Property Field() As String
        ...
    End Property

End Class

skagen00 replied on Tuesday, August 29, 2006

I have to admit I haven't really given this any thought, but is there a way to leverage generics to enable the target to be of the appropriate type and avoid the need to do a cast?

Edit // Nevermind!