Thanks for the new release. Looks like there are some nice features in
there. However, I had written some of my own unit tests on for the previous
version and found that 4 of the tests broke with the new release. It seems
that when I have two transient objects with the domain signature properties
of FirstName and LastName, if I set one objects properties to FName1 and
LName1 and a different objects properties to FName2 and LName2, the result
is coming back as true when they are compared. If I change the different
object property values to something like diffname and difflastname the
comparison returns false as expected. Am I writing this test wrong or is
this the expected behavior? Here is the Unit Test code that I am using that
has brought about the issue. If you change the _diffObj and the
_diffObjWithId FirstName and LastName properties the tests will run fine.
Thanks again for this great architecture.
Brian
using NUnit.Framework;
using SharpArch.Core;
using SharpArch.Core.PersistenceSupport;
namespace Tests.SharpArch.Core.PersistenceSupport
{
[TestFixture]
[Category("Unit")]
public class MyPersistentObjectTests
{
private MockPersistentDomainObjectWithDefaultId _obj;
private MockPersistentDomainObjectWithDefaultId _sameObj;
private MockPersistentDomainObjectWithDefaultId _diffObj;
private MockPersistentDomainObjectWithSetId _objWithId;
private MockPersistentDomainObjectWithSetId _sameObjWithId;
private MockPersistentDomainObjectWithSetId _diffObjWithId;
[SetUp]
public void Setup()
{
_obj = new MockPersistentDomainObjectWithDefaultId
{
FirstName = "FName1",
LastName = "LName1",
Email = "test...@mail.com"
};
_sameObj = new MockPersistentDomainObjectWithDefaultId
{
FirstName = "FName1",
LastName = "LName1",
Email = "test...@mail.com"
};
_diffObj = new MockPersistentDomainObjectWithDefaultId
{
FirstName = "FName2",
LastName = "LName2",
Email = "testu...@mail.com"
};
_objWithId = new MockPersistentDomainObjectWithSetId
{
FirstName = "FName1",
LastName = "LName1",
Email = "test...@mail.com"
};
_sameObjWithId = new MockPersistentDomainObjectWithSetId
{
FirstName = "FName1",
LastName = "LName1",
Email = "test...@mail.com"
};
_diffObjWithId = new MockPersistentDomainObjectWithSetId
{
FirstName = "FName2",
LastName = "LName2",
Email = "testu...@mail.com"
};
}
[TearDown]
public void Teardown()
{
_obj = null;
_sameObj = null;
_diffObj = null;
}
[Test]
[Category("Unit")]
public void
DoesDefaultPersistentObjectEqualsOverrideWorkWhenNoIdIsAssigned()
{
Assert.That(_obj.Equals(_sameObj));
Assert.That(!_obj.Equals(_diffObj));
Assert.That(!_obj.Equals(new
MockPersistentDomainObjectWithDefaultId()));
}
[Test]
[Category("Unit")]
public void
DoEqualDefaultPersistentObjectsWithNoIdsGenerateSameHashCodes()
{
Assert.That(_obj.GetHashCode().Equals(_sameObj.GetHashCode()));
}
[Test]
[Category("Unit")]
public void
DoEqualDefaultPersistentObjectsWithMatchingIdsGenerateDifferentHashCodes()
{
Assert.That(!_obj.GetHashCode().Equals(_diffObj.GetHashCode()));
}
[Test]
[Category("Unit")]
public void
DoDefaultPersistentObjectEqualsOverrideWorkWhenIdIsAssigned()
{
_obj.SetAssignedIdTo(1);
_diffObj.SetAssignedIdTo(1);
Assert.That(_obj.Equals(_diffObj));
}
[Test]
[Category("Unit")]
public void DoesPersistentObjectEqualsOverrideWorkWhenNoIdIsAssigned()
{
Assert.That(_objWithId.Equals(_sameObjWithId));
Assert.That(!_objWithId.Equals(_diffObjWithId));
Assert.That(!_objWithId.Equals(new
MockPersistentDomainObjectWithSetId()));
}
[Test]
[Category("Unit")]
public void DoEqualPersistentObjectsWithNoIdsGenerateSameHashCodes()
{
Assert.That(_objWithId.GetHashCode().Equals(_sameObjWithId.GetHashCode()));
}
[Test]
[Category("Unit")]
public void
DoEqualPersistentObjectsWithMatchingIdsGenerateDifferentHashCodes()
{
Assert.That(!_objWithId.GetHashCode().Equals(_diffObjWithId.GetHashCode()));
}
[Test]
[Category("Unit")]
public void DoPersistentObjectEqualsOverrideWorkWhenIdIsAssigned()
{
_objWithId.SetAssignedIdTo("1");
_diffObjWithId.SetAssignedIdTo("1");
Assert.That(_objWithId.Equals(_diffObjWithId));
}
}
public class MockPersistentDomainObjectWithSetId :
MockPersistentDomainObjectBase<string>, IHasAssignedId<string>
{
public void SetAssignedIdTo(string assignedId)
{
ID = assignedId;
}
}
public class MockPersistentDomainObjectWithDefaultId :
MockPersistentDomainObjectBase, IHasAssignedId<int>
{
public void SetAssignedIdTo(int assignedId)
{
ID = assignedId;
}
}
public class MockPersistentDomainObjectBase :
MockPersistentDomainObjectBase<int> { }
public class MockPersistentDomainObjectBase<T> :
PersistentObjectWithTypedId<T>
{
[DomainSignature]
public string FirstName { get; set; }
[DomainSignature]
public string LastName { get; set; }
public string Email { get; set; }
}
}
If I'm not mistaken, the only change that has happened is that now you'll
get into domain business signature comparison only if both objects are
transient or not (simultaneously)...
--
Luis Abreu
> No virus found in this incoming message.
> Checked by AVG - http://www.avg.com
> Version: 8.0.173 / Virus Database: 270.7.5/1698 - Release Date: 01-10-
> 2008 09:05
private class ObjectWithIntId2 : PersistentObject {
[DomainSignature]
public string Name { get; set; }
[DomainSignature]
public string Address { et; set; }
}
Now, the test:
[Test]
public void ShouldBeDifferent()
{
var obj1 = new ObjectWithIntId2 {Name = "A", Address = "B"};
var obj2 = new ObjectWithIntId2 { Name = "B", Address = "A" };
Assert.That(obj1, Is.Not.EqualTo(obj2));
}
Ok, i think that we need to run equals on the properties instead of using
the hashcode. What do you guys think?
--
Luis Abreu
> Version: 8.0.173 / Virus Database: 270.7.5/1702 - Release Date: 01-10-
> 2008 09:05
Brian
--
Luis Abreu
> > Version: 8.0.173 / Virus Database: 270.7.5/1702 - Release Date: 01-
_sameObj = new transientObject
{
FirstName = "FName1",
LastName = "LName1",
Email = "test...@mail.com"
};
_diffObj = new transientObject
{
FirstName = "FName2",
LastName = "LName2",
Email = "testu...@mail.com"
};
------
so, we do need some way to corretly implement hascode...anyone good with math?
--
Regards,
Luis Abreu
I guess that the problem is that we're thinking about the GetHashcode
method incorrectly. Ok, if 2 objects are equal, the hashcode must be
equal, but if two objects are different, there's nothing out there
that says that they must return a different hashcode! So, the problem
here is that we're using the hascode for equality when we shouldn't be
doing that. Even with a fancy algorythm, there's always the chance of
having 2 "different" objects returning the same hashcode.
So, I'd really prefer to add a method that compares properties instead
of relying in hashcode. For instance, adding a method like this:
protected bool DoObjectsHaveSomeBusinessSignature(DomainSignatureComparable to)
{
Boolean hasDomainSignatureSpecified = false;
foreach (PropertyInfo property in GetType().GetProperties())
{
if (IsPartOfDomainSignature(property))
{
hasDomainSignatureSpecified = true;
object value = property.GetValue(this, null);
object anotherValue = property.GetValue(to, null);
if( !value.Equals(anotherValue) )
{
return false;
}
}
}
return true & hasDomainSignatureSpecified;
}
And then I'd change the DomainSignatureComparable Equals to:
public override bool Equals(object obj) {
DomainSignatureComparable compareTo = obj as
DomainSignatureComparable;
return compareTo != null &&
DoObjectsHaveSomeBusinessSignature(compareTo);
}
and, of course, the Equals method oe PersistentObject to:
public override bool Equals(object obj) {
PersistentObjectWithTypedId<IdT> compareTo = obj as
PersistentObjectWithTypedId<IdT>;
return compareTo != null &&
(HasSameNonDefaultIdAs(compareTo) ||
// Since the IDs aren't the same, both of them
must be transient to
// compare business value signatures. If one is
transient and the other is
// a persisted entity, then they cannot be the same object.
(((IsTransient()) && compareTo.IsTransient()) &&
DoObjectsHaveSomeBusinessSignature(compareTo)));
}
Ok, this breaks the domainscignaturecomparable tests (only!). no doubt
about it. However. i believe that this is a good thing because I'm not
sure about these tests. For instance, the
CanCompareDomainObjectsWithOnlySomePropertiesBeingPartOfDomainSignature
looks like this:
public void CanCompareDomainObjectsWithOnlySomePropertiesBeingPartOfDomainSignature()
{
ObjectWithOneDomainSignatureProperty object1 = new
ObjectWithOneDomainSignatureProperty();
ObjectWithOneDomainSignatureProperty object2 = new
ObjectWithOneDomainSignatureProperty();
Assert.That(object1, Is.Not.EqualTo(object2));
object1.Age = 13;
object2.Age = 13;
// Name property isn't included in comparison
object1.Name = "Foo";
object2.Name = "Bar";
Assert.That(object1, Is.EqualTo(object2));
object1.Age = 14;
Assert.That(object1, Is.Not.EqualTo(object2));
}
Now, why do we have the first assertion? I mean,
DomainSignatureComparable does not have the concept of transitory
objects. So, I think that the 1st Assert is wrong. Here's how I expect
this class to behave:
1. if there isn't any property anotated with the domainsignature
attribute, then I expect it to return false (always). I assume this
because in this case equality is synonim of having the same values
applied to predefined properties. Ok, there's one case where I think
some debate is in order: what if we have two references to the same
object of a type that doesn't has any property used in the domain
signature? Ideas? Btw, it could be useful to optimize the code by
using Object.ReferenceEquals in my previous code suggestion. if it
returns true, then there's no need of going throught reflection,
right?
2. if the class does have domain signature properties, then the
equality should be true if all those properties have the same value.
Now, if you look at the previous test, that is true: the age property
is part of the signature and since the class doesn't has the concept
of transient (delegate to the persistenobjectXXX class), then that
assert shouldn't hold. ie, two new instances will get the same value
for their ID, which means that they should be equal. Note that I'm
talking about the domainsignaturecomparable class, not anout the
persistenobjectXXX class. If the DomainSignatureComparable class had
the notion of transient object, then I would expect that assert to
hold...
so, what do you think?
--
Regards,
Luis Abreu
public abstract class DomainSignatureComparable {
public override bool Equals(object obj) {
var compareTo = obj as DomainSignatureComparable;
if (ReferenceEquals(this, compareTo)) return true;
return compareTo != null &&
DoObjectsHaveSomeBusinessSignature(compareTo);
}
/// <summary>
/// Used to compare two objects via <see cref="Equals"/>;
although it's necessary for
/// NHibernate's use, this may also be useful for business
logic purposes.
/// </summary>
public override int GetHashCode() {
// Since it's possible for two objects to return the same
domain signature,
// even if they're of two different types, it's important
to include the object's
// type in the domain signature
return GetType().GetHashCode() ^ GetDomainObjectSignature();
}
/// <summary>
/// The method GetDomainObjectSignature should ONLY return the
"business value
/// signature" of the object and not the ID, which is handled
by <see cref="PersistentObject" />.
///
/// If you choose NOT to override this method, then you should
decorate the appropriate
/// property(s) with [DomainSignature] and they will be
compared automatically. Using the
/// [DomainSignature] attribute is the preferred method.
///
/// Alternatively, if you override this method, the general
structure of the overridden method
/// should be as follows:
///
/// return SomeProperty.GetHashCode() ^ SomeOtherProperty.GetHashCode();
///
/// </summary>
protected virtual int GetDomainObjectSignature() {
var domainObjectSignature = 0;
foreach (var property in GetType().GetProperties()) {
if (!IsPartOfDomainSignature(property))
{
continue;
}
var value = property.GetValue(this, null);
if (value != null) {
domainObjectSignature = domainObjectSignature ^
value.GetHashCode();
}
}
// If no properties were flagged as being part of the
domain signature of the object,
// then simply return the hashcode of the base object as
the domain signature. This
// behaves as you would normally expect Equals to behave
when comparing two objects.
return domainObjectSignature == 0 ? base.GetHashCode() :
domainObjectSignature;
}
protected bool
DoObjectsHaveSomeBusinessSignature(DomainSignatureComparable to) {
var hasDomainSignatureSpecified = false;
foreach (var property in GetType().GetProperties()) {
if (!IsPartOfDomainSignature(property))
{
continue;
}
hasDomainSignatureSpecified = true;
var value = property.GetValue(this, null);
var anotherValue = property.GetValue(to, null);
if(ReferenceEquals(value, anotherValue) ) return true;
if( value == null || anotherValue == null) return false;
if (!value.Equals(anotherValue)) {
return false;
}
}
return true & hasDomainSignatureSpecified;
}
private static Boolean IsPartOfDomainSignature(PropertyInfo property) {
return
property.GetCustomAttributes(typeof(DomainSignatureAttribute),
true).Length > 0;
}
}
--
Regards,
Luis Abreu
protected virtual int GetDomainObjectSignature()
{
var domainObjectSignature = 0;
var strValues = new StringBuilder();
foreach (var property in GetType().GetProperties())
{
if (!IsPartOfDomainSignature(property))
{
continue;
}
var value = property.GetValue(this, null);
if (value != null)
{
strValues.Append(value.ToString());
}
}
domainObjectSignature = strValues.ToString().GetHashCode();
// If no properties were flagged as being part of the domain signature
of the object,
// then simply return the hashcode of the base object as the domain
signature. This
// behaves as you would normally expect Equals to behave when comparing
two objects.
return strValues.ToString().GetHashCode == 0 ? base.GetHashCode() :
domainObjectSignature;
}
By appending the string values to a single hash code it seems to return a
unique code in the test case that I had previously had problems with. I
still agree that we should not rely on the hash code to determine property
equality though.
Most of the implementations I've seen are really happy delegating to its
inner properties, so I'd leave it like that...
Anyways, we must wait for Billy to see what he thinks about the ideas we've
presented here.
--
Luis Abreu
> -----Original Message-----
> From: sharp-arc...@googlegroups.com [mailto:sharp-
> archit...@googlegroups.com] On Behalf Of Brian Nicoloff
> Sent: quinta-feira, 2 de Outubro de 2008 16:58
> To: sharp-arc...@googlegroups.com
> Subject: RE: Equals and Hashcode question in new release?
>
>
> Version: 8.0.173 / Virus Database: 270.7.5/1702 - Release Date: 02-10-
> 2008 07:46
Just one question (probably didn't get this right):
> * Two PersistentObject/PersistentObjectWithTypedId objects which have
> the same ID are always equal;
So, are you using the db key for checking equality between objects? Are we
dismissing the attributes annotated with the DomainSignatureAttribute from
this comparison? If so, is this wise?
Luis
Probably, but the question is: does it work in all the scenarios?
--
Luis Abreu
No
virus found in this incoming message.
Checked by AVG - http://www.avg.com
Version: 8.0.173 / Virus Database: 270.7.5/1703 - Release Date: 05-10-2008 09:20
I'll have to think a little more about that to see if I agree. I mean,
it makes sense, but you'll also need to garantee that nobody changes
the value of the ID after its set....need to think a little more about
it....
--
Regards,
Luis Abreu
I'm seeing that hashcodes are still being used for transient
comparisions...didn't we reach the conclusion that was not correct?
the last time I've looked, there really wasn't anything that stated
that 2 different objects couldn't return the same hash code. So. why
don't we jut compare the domain signature properties?
regarding the ID, i guess that it will work ok if you can ensure that
you won't be getting 2 different objects with the same id and
different business signature values. not sure if that will work all
the times, but if you're controlling access to the db, that will
probably work.
there's still one thing I'd like to see in the framework: version timestamps...
--
Regards,
Luis Abreu
--
Regards,
Luis Abreu
can't remember anything right now :)
the only thing that i know is that implementing equality based on
hashing is completely wrong specially because the only think you know
for sure is that if 2 objects are equal, they should produce the same
hash, but nothing is said regarding difference...
--
Regards,
Luis Abreu
--
Regards,
Luis Abreu
Ian Griffiths blog has an interesting post on this subject:
http://www.interact-sw.co.uk/iangblog/2004/06/21/gethashcode
So, I'd say that implementing Hashcode is more difficult than it might
seem on first sight. I think that this confirms the fact that equality
should never be based on the hash and that we must probably think a
little more on how to implement hashcodes...
--
Regards,
Luis Abreu
--
Regards,
Luis Abreu