I'm not sure exactly when CSLA changed from using value equality to reference equality in BusinessBase (I think it was in 3.0 but it might have been back in 2.1). We elected to stay with value equality in our Equals() overload.
However, I notice that the FieldManager does not use reference equality when you are setting or loading properties and will discard attempts to supply a different reference if the replacement object has the same id. At least on the surface, it seems to me that if I supply an entirely different reference to an object to a property setter, then it should treat that as a property change even I've chosen to overload Equals().
Would it be possible to change LoadProperty and SetProperty to use object.ReferenceEquals() to detect property changes? If not, I'll certainly have to abandon value equality in our objects. This will be a massive breaking change, but we can't have the property setters ignore objects that have made a round-trip through the data portal, as this is causing very hard to understand problems.
I stared at a 2 lines of code yesterday for at least one hour and couldn't figure out how a local variable containing the reference and the BOproperty I had just assigned it to could possibly have different data in them until I remembered the value equality wrinkle.
This is in regard to a child object reference I assume? If so this seems like a reasonable change.
Is your app a web app or Windows Forms? If Windows Forms, eventually you'll need to abandon value equality anyway, because you can't stay on Windows Forms forever - I strongly expect that over the next few years it'll become increasingly hard to find developers, etc.
Thanks, Rocky. Yes, this is a child object reference I was having trouble with.
As I recall, you made the change to the BusinessBase Equals() implementation to support WPF, but at the time it was a breaking change so I copied the previous Equals() implementation in our own class library. I was using object.ReferenceEquals() to detect changes in my own property setters before moving to managed child properties, so this didn't show up immediately.
I know we have to move to reference equality eventually -- I just need to get my head wrapped around exactly what we will need to change to make that work. (It's quite doable, I just have to plan for it)
Hi,
I have been assigned this bug and have checked in the necessary cheanges in code for BusinessBase in both Csla 3.8 and 4.0.
http://lhotka.net/cslabugs/edit_bug.aspx?id=731
Grab the latest code and test it on your app. Code will now use ReferenceEquals if PropertyInfoType is assignable from IBusinessObject.
Wow! Fixed in less than 24 hours after I reported it. Very impressive --thanks.
I had tentatively implemented a very similar fix in my own copy but some research prompted some questions (my apologies if I'm straying slightly off topic). My fix looked like this:
if (oldValue == null)
valuesDiffer = newValue != null;
// CHANGE TO CSLA HERE
// Use Reference Equality to detect changes in reference types
else if (propertyInfo.Type.IsValueType)
valuesDiffer = !(oldValue.Equals(newValue));
else
valuesDiffer = !object.ReferenceEquals(oldValue, newValue);
I was just wondering -- why wouldn't something very simple like the following one-liner work here? Wouldn't this implicitly use value equality for value types and reference equality for other types?
valuesDiffer = newValue != oldvalue;
Yes, I started out with that same fix - but I know there a several Smart..... datatypes being used (SmartDate and several others available from CslaContrib) and so to minimize the risk of introducing new bugs I decided to test for IBusinessObject instead as these are the typical BusinessObjects.
Sorry, haven't had time to test for the last code you presented here.
Yes, I started out with that same fix - but I know there a several Smart..... datatypes being used (SmartDate and several others available from CslaContrib) and so to minimize the risk of introducing new bugs I decided to test for IBusinessObject instead as these are the typical BusinessObjects.
Sorry, haven't had time to test for the last code you presented here.
I understand completely -- the fix you implemented has the smallest possible impact while still solving the problem I reported. I'm all for being conservative especially in a part of the framework where making a mistake has the potential to have very broad impact.
Thanks!
I just downloaded the 3.8.3 beta, and I don't see a change to SetProperty() for this problem. I do see where LoadProperty() has been changed to use reference equality, but SetProperty() is still using the .Equals() instance function.
Copyright (c) Marimer LLC