Sorry for the catchy subject, (you can read the story behind this a bit later), but there is a benefit out of the above statement by modifying BusinessBase::Equals and ReadOnlyBase::Equals accordingly to match the following.
public override bool Equals(object obj)
{
if (Object.ReferenceEquals(obj, this))
{
return true;
}
if (obj is T)
{
object id = GetIdValue();
if (id == null)
throw new ArgumentException(Resources.GetIdValueCantBeNull);
return id.Equals(((T)obj).GetIdValue());
///return Object.ReferenceEquals(id,((T)obj).GetIdValue());
}
else
return false;
}
The difference is the added “ReferenceEquals” block.
This might not seem to be a real benefit at first, but “Equals” is called quite often from the Data Binding mechanism, probably for checking the equality between a data-source object of a control and the parent object of a bound method. So, most of the time the “obj” parameter and the “this” pointer refer to the very same object (I have only checked in Windows Forms UI though).
It would also be nice to update the documentation of “GetIdValue” to inform users of the library not to return the “this” pointer for identifying an object!
The bad thing in returning the “this” pointer for identifying an object is that the “GetIdValue” is called from the overridden “GetHashCode()” and “ToString()” methods. For these, I think it should be documented that they would be overridden in such implementation.
On the other hand, the framework might forgive the lazy ones and resolve the recursive call internally. Here is a possible approach. Keep in mind that these methods are not called all the time.
public override int GetHashCode()
{
object id = GetIdValue();
if (id == null)
throw new ArgumentException(Resources.GetIdValueCantBeNull);
if (!Object.ReferenceEquals(id, this))
{
return id.GetHashCode();
}
return base.GetHashCode();
}
public override int ToString()
{
object id = GetIdValue();
if (id == null)
throw new ArgumentException(Resources.GetIdValueCantBeNull);
if (!Object.ReferenceEquals(id, this))
{
return id.ToString();
}
return base.ToString();
}
Thank you for your patience
Ioannis
The Story starts here.
We had a design where some business objects had to use some transient data for as long as they were active in memory. The user could modify these data during the active session.
It was the best candidate for an Undoable, NonSerialized and Editable child object. So, since there was no need for permanently storing this child object – I have a C++ background – I assumed that returning the “this” pointer would be the best identification value for the Object. (Unfortunately in .Net “this” is the object itself and not some memory address placeholder).
The code was like this. (I presume the similarity to C# is recognized)
[Serializable]
Public ref class TransientInfo : public Csla::BusinessBase<TransientInfo>
{
//… code
public:
virtual Object^ GetIdValue() override
{
return this;
}
//… code
};
A colleague designed a User Control to edit the properties of the object using data binding. Unfortunately, when we tested the solution, by the moment we tabbed out of any editable field the application crashed. Even in the debugger, the visual studio would exit debugging mode with no reason.
After too many breakpoints, checking with several versions of Csla and countable hours, we found out that the last known point before the debugger crashed was BusinessBase::Equals().
public override bool Equals(object obj)
{
if (obj is T)
{
object id = GetIdValue();
if (id == null)
throw new ArgumentException(Resources.GetIdValueCantBeNull);
return id.Equals(((T)obj).GetIdValue());
///return Object.ReferenceEquals(id,((T)obj).GetIdValue());
}
else
return false;
}
It turned out that the failing point was this line.
return id.Equals(((T)obj).GetIdValue());
Since we returned (this) from our GetIdValue() we had caused an infinite, recursive loop at this point!!!
My first reaction was to return the result of GetHashCode() as the identifying value for our objects. This is also overridden in BusinessBase though, so, we got to the same result. The final test was to temporarily return the constant value (-1), which resolved our case.
The point is, even though we were returning a constant value, the “obj” parameter of (Equals) was still the SAME child object. (this object!!!). It was evident by that moment (a bit later actually) that Equals was called through the data binding mechanism. We got the same behavior when modifying the parameters of the parent Object. So the old C++ idiom for checking the address of the this pointer (&this == &objParam) would be a good candidate for this method.
So we modified the BusinessBase::Equals and ReadOnlyBase::Equals methods and we added the missing “id” field in the object! Overriding the GetHashCode() and ToString() methods as described above would be too much for this.
This was our final implementation.
[Serializable]
Public ref class TransientInfo : public Csla::BusinessBase<TransientInfo>
{
private:
TransientInfo()
{
_id = Object::GetHashCode(); //==(base.GetHashCode())
}
//… code
private:
[NotUndoable()]
[NonSerialized()]
int _id;
//… code
public:
virtual Object^ GetIdValue() override
{
return _id;
}
//… code
};
Copyright (c) Marimer LLC