Hi all,
I've a subclass of BusinessBase(Of T) wich has as sub listening for the ValidationRules.GetBrokenRules.ListChanged event.
If the argument e.ListChangedType equals ComponentModel.ListChangedType.ItemAdded then I must check the IsValid value. However this value is not correct inside this event handler.
I've tracked the problem to this method in BrokenRulesCollection.vb:
Sub Add(ByVal rule As IRuleMethod)
...
Add(item)
IncrementCount(item)
...
IsValid depends on mErrorCount and this variable gets incremented on IncrementCount. But the ItemAdded event gets fired before this increment so the IsValid value is not correct within the event handler context.
The Remove Sub is working because it's decrementing this counter before removing the item.
So I think these two lines should be swapped, but I don't know if that could break existing code. What do you think?
Jacobo
I'm using ActiveObjects 2.0 and the ActiveBusinessBase class listens to that event so it can fire an observable IsValid event. This is the code :
Private Sub BrokenRulesListChangedHandler(ByVal sender As Object, ByVal e As System.ComponentModel.ListChangedEventArgs) Select Case e.ListChangedType Case ComponentModel.ListChangedType.ItemAdded, ComponentModel.ListChangedType.ItemDeletedNotify(BusinessEvents.IsValid,
Me.IsValid)
Well I already have changed it and have my code working without problems. This post is more a suggestion to Rocky to swap those two statements and fix this tiny bug.
This change would also match the Remove implementation (decrementing then removing item) as I said.
As for the ActiveObjects dependency on CSLA I don't see that it's doing anything illegal, it's just subclassing businessbase and listening an event from the protected variable ValidationRules. I could have another scenario without AO at all, and still reproduce the issue.
(BTW I think this problem was introduced with CSLA 2.1, because since then IsValid is based on the mErrorCount variable rather than the .Count method)
Regards
I think the issue with swapping them arround is that is the Add fails, the count will then be incorrect. Whereas at the moment if the add fails then the counter wont be incremented? I'm not 100% sure about this, but its the first thing I noticed when looking at the code your posted.
Thinking about that, would this mean that if the remove failed the count would also be incorrect? could the remove ever fail?
Just a thought
Craig
Hi Craig,
You're correct, if the Add or Remove methods fail the counter would get unsynchronized. I don't know if this could ever happen but I think that is very improbable because we're talking about the BrokenRulesCollection class wich is wrapped inside ValidationRules. So the Add/Remove code is a little more 'protected' from the user because we normally don't interact directly with BrokenRulesCollection ...
Anyway I still think that those lines should be swapped in the Add sub, at least to expose the same behavior in both methods
amselem:
You're correct, if the Add or Remove methods fail the counter would get unsynchronized. I don't know if this could ever happen ...
Well I already have changed it and have my code working without problems. This post is more a suggestion to Rocky to swap those two statements and fix this tiny bug.
Again, there could be a reason for the order in which the statements are executed. I'm not saying for sure there is, we don't know...This change would also match the Remove implementation (decrementing then removing item) as I said.
Its depending on the fact that the ListChanged event is only called after the counters are updated. Now, that's probably a valid assumption, and unless there's a good reason the code should probably be changed, but it still seems like AO is depending on how the Add method is implemented.As for the ActiveObjects dependency on CSLA I don't see that it's doing anything illegal, it's just subclassing businessbase and listening an event from the protected variable ValidationRules. I could have another scenario without AO at all, and still reproduce the issue.
(BTW I think this problem was introduced with CSLA 2.1, because since then IsValid is based on the mErrorCount variable rather than the .Count method)
Hi John
Yes, I'm still using this technique and didn't have any issues after this post. This is the relevant code from my BusinessBase subclass, just don't forget to add/remove the handler when de/serializing the object :
Protected Sub New()
MyBase.new()
Me.AddHandlers()
End Sub
Protected Overrides Sub OnDeserialized(ByVal context As StreamingContext)
MyBase.OnDeserialized(context)
Me.AddHandlers()
End Sub
<OnSerialized()> _
Protected Sub OnSerialized(ByVal context As StreamingContext)
Me.RemoveHandlers()
End Sub
Private Sub AddHandlers()
' Handle events raised by our BrokenRules
AddHandler ValidationRules.GetBrokenRules.ListChanged, AddressOf BrokenRulesListChangedHandler
End Sub
Private Sub RemoveHandlers()
' Handle events raised by our BrokenRules
RemoveHandler ValidationRules.GetBrokenRules.ListChanged, AddressOf BrokenRulesListChangedHandler
End Sub
Private mOldIsValidState As Boolean = False
Private mIsValidStateInitialized As Boolean = False
Private Sub BrokenRulesListChangedHandler(ByVal sender As Object, ByVal e As System.ComponentModel.ListChangedEventArgs)
Select Case e.ListChangedType
Case ComponentModel.ListChangedType.ItemAdded, ComponentModel.ListChangedType.ItemDeleted
Dim newIsValidState As Boolean = Me.IsValid
' Check if our IsValid state has changed (only our own, not of child objects too)
If (Not mIsValidStateInitialized) OrElse (mOldIsValidState <> newIsValidState) Then
mIsValidStateInitialized = True
mOldIsValidState = newIsValidState
' fire the IsValid event since our IsValid state has changed
Notify(BusinessEvents.IsValid, newIsValidState)
End If
End Select
End Sub
HTH
amselem
Copyright (c) Marimer LLC