Version 0.8.0 with MVC Preview 5, NH 2.0 and Ninject 1.0 now available!

200 views
Skip to first unread message

Billy

unread,
Oct 1, 2008, 1:08:04 AM10/1/08
to S#arp Architecture
After a long delay, I finally have 0.8 available for download which
includes the official release of S#arp Architecture using MVC Preview
5, NHibernate 2.0 and Ninject 1.0.

Here is a summary of some of the more significant changes (some
breaking changes) in this release:
* Documentation has been completely updated and much new content has
been added
* ProjectBase has been renamed to SharpArch
* Ninject is now the official mechanism for performing dependency
injection
* The Equals/GetHashCode has been overhauled to be much faster;
additionally, it no longer sees a transient and persistent object as
being the same if their domain signatures are the same
* The basic Equals/GetHashCode logic has been pulled up into the
abstract class SharpArch.Core/DomainSignatureComparable.cs; this may
be used as a base class for non-persistent domain classes to provide
consistent Equals/GetHashCode behavior, e.g. an Address object mapped
as a component of an Employee
* Unit tests have been added to the SharpArch solution; it's not
comprehensive yet, but it's a start

I feel that this is the first major release with a codebase which will
no longer be changing much. Most of the changes which will be coming
in future releases will be upgrades to third party components such as
ASP.NET MVC and NHibernate, inclusion of support for new areas of
functionality such as WCF, and more sample code. So it is my
intention that future releases will have very few breaking changes.

Billy McCafferty

Brian Nicoloff

unread,
Oct 1, 2008, 1:56:34 PM10/1/08
to sharp-arc...@googlegroups.com
Billy,

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; }
}
}


Luis Abreu

unread,
Oct 1, 2008, 2:07:25 PM10/1/08
to sharp-arc...@googlegroups.com
Hello.

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

Brian Nicoloff

unread,
Oct 1, 2008, 2:21:52 PM10/1/08
to sharp-arc...@googlegroups.com
That's what I thought too, but these test cases are showing otherwise. They
ran fine prior to this release. It seems that the hashcode is returning the
same value for the two objects when the domain signatures are off by 1
character. For instance when the FirstName property value is set to FName1
on one object and the property is set to FName2 on the other object, the
hashcode is returning the same value.

Luis Abreu

unread,
Oct 1, 2008, 4:37:13 PM10/1/08
to sharp-arc...@googlegroups.com
Well, I can see you get a false positive by exchanging values on properties
of the same type. Billy, what do you think about this example:

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 Nicoloff

unread,
Oct 1, 2008, 5:09:31 PM10/1/08
to sharp-arc...@googlegroups.com
I agree with that. I am by no means an expert on how the hash code
determines equality, but with your example and the previous one that I
uncovered, it seems that there is definitely a problem when comparing 2
objects based on their [DomainSignature] properties.

Brian

Luis Abreu

unread,
Oct 1, 2008, 5:29:38 PM10/1/08
to sharp-arc...@googlegroups.com
Yes, but I'm not sure your case is similar to mine. After all, you have 2
different values while I have the same values swapped (which is why I'm
getting the wrong result on my test)....

--
Luis Abreu

> > Version: 8.0.173 / Virus Database: 270.7.5/1702 - Release Date: 01-

Brian Nicoloff

unread,
Oct 1, 2008, 5:47:53 PM10/1/08
to sharp-arc...@googlegroups.com
It could be two separate problems I guess. Tracing through my test code
points to the same issue with the hash code that you suggested though. Both
property values are returning the same hash code when they are similar. It
seems that if the code checked for property equality rather than hash code
it will work.

Luis Abreu

unread,
Oct 1, 2008, 5:50:55 PM10/1/08
to sharp-arc...@googlegroups.com
Hum...that is interesting...I mean, in my case, it was obvious I'd get the
same hash because I?ve just exchanged the properties' values, but in yours
you had different values (ie, you f1/l1 and then f2/l2 - or something along
these lines). I expected that the hashcode for these would always be
different, but again, I might be wrong.

Brian Nicoloff

unread,
Oct 1, 2008, 6:11:21 PM10/1/08
to sharp-arc...@googlegroups.com
Yes, that was my assumption of the way that the hash code worked as well.
But, running the test through the debugger brings up the same HashCode in
the GetDomainObjectSignature() method for the following objects where
FirstName and LastName are the combined DomainSignature for the class.

_sameObj = new transientObject


{
FirstName = "FName1",
LastName = "LName1",
Email = "test...@mail.com"
};

_diffObj = new transientObject


{
FirstName = "FName2",
LastName = "LName2",
Email = "testu...@mail.com"
};

------

Luis Abreu

unread,
Oct 2, 2008, 5:26:37 AM10/2/08
to sharp-arc...@googlegroups.com
incredible...I've just run some powershell tests and yes, you're
correct. Interestingly, each string has its own integer value but when
youo combine them with ^operator you get the same result...

so, we do need some way to corretly implement hascode...anyone good with math?

--
Regards,
Luis Abreu

Martin Hornagold

unread,
Oct 2, 2008, 5:43:09 AM10/2/08
to sharp-arc...@googlegroups.com
Hi guys and Billy,

Just did a quick hunt and found a useful article on the same problem
documented for Java:

http://mindprod.com/jgloss/hashcode.html

Relevant quote from the article:

The nasty properties of XOR are:
It treats identical pairs of components as if they were not there.
It ignores order. A ^ B is the same a B ^ A. If order matters, you want
some sort of checksum/digest such as Adlerian. XOR would not be suitable
to compute the hashCode for a List which depends on the order of the
elements as part of its identity.
If the values to be combined are only 8 bits wide, there will be
effectively only 8 bits in the xored result. In contrast, multiplying by
a prime and adding would scramble the entire 32 bits of the result,
giving a much broader range of possible values, hence greater chance
that each hashcode will unique to a single Object. In other words, it
tends to expand the range of the digest into the widest possible band
(32 bits).


There are some examples of implementation.

Hope it helps

Martin

Luis Abreu

unread,
Oct 2, 2008, 6:10:08 AM10/2/08
to sharp-arc...@googlegroups.com
Hello guys.

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

Luis Abreu

unread,
Oct 2, 2008, 6:48:31 AM10/2/08
to sharp-arc...@googlegroups.com
small correction to the previous snippets:

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

Brian Nicoloff

unread,
Oct 2, 2008, 11:58:08 AM10/2/08
to sharp-arc...@googlegroups.com
Luis,
I agree with both of your statements below regarding the
DomainSignatureComparable behavior. I think that the Object.ReferenceEquals
handles the situation where we have two references to the same object of a
type that doesn't have any property used in the domain. My only change to
the code that you provided would be to possibly make the hash code a little
more unique by changing the GetDomainObjectSignature method to the
following:

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.

Luis Abreu

unread,
Oct 2, 2008, 3:55:32 PM10/2/08
to sharp-arc...@googlegroups.com
Hum...well, calculating a hashcode is...you know, hard:)

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

Brian Nicoloff

unread,
Oct 2, 2008, 5:39:40 PM10/2/08
to sharp-arc...@googlegroups.com
Sounds good. Thanks for your help.

Simone Busoli

unread,
Oct 4, 2008, 6:40:51 PM10/4/08
to sharp-arc...@googlegroups.com
Billy, is the project going to be a one man show forever? Just to know if I, or anyone, should bother discussing changes, fixes or improvements.

Billy

unread,
Oct 5, 2008, 1:46:07 AM10/5/08
to S#arp Architecture
No! In fact, the changes that I just checked in (which I'll be making
0.8.1) include most of the patch which you've provided (and probably
thought that I did not notice)...so thank you Simone for the
contribution!! Additionally, I'd like to sincerely thank Brian
Nicoloff (especially for the comprehensive unit tests), Luis Abreu,
and Martin Hornagold for this discussion which exposed a serious flaw
in the existing logic and provided terrific discussion for the
applicability of Equals and GetHashCode, and the logic therein. The
fact of the matter is, this architecture, and current release, is a
true reflection of everyone who has participated within these
discussions. I only wish to keep the check in/out of the core code
base between now and 1.0 to myself and Frank Laub, who has been
instrumental in the formalization of this architecture, for the sole
motivation that I wish to consider the feedback from all developers
and incorporate the guidance which is most in line with my own
(arguably selfish) goals for where I want this architecture to be and
how I want it to be used. My aspiration is that S#arp Architecture
will become the "Ruby on Rails" foundation for ASP.NET MVC; I feel
that we will get it there with all of your input and assistance.

The most recent check-in (which will be part of 0.8.1) includes the
following assumptions with respect to Equals and GetHashCode:
* Equals and GetHashCode should behave identically (this is up for
argument from a theoretical standpoint but an assumption for #arch).
Similar to the guidance provided within
http://www.hibernate.org/hib_docs/nhibernate/html/persistent-classes.html#persistent-classes-equalshashcode
, the code with #arch leverages the GetHashCode method to enable
checks for equality; #arch is just a bit more explicit about the
assumed overlap;
* If two objects inherit from DomainSignatureComparable, and are
compared, then it is assumed that the comparison between the two is
solely dependent on the properties marked with the attribute
[DomainSignature]. If no properties are decorated with the
DomainSignature attribute, then the comparison between the two objects
is treated as .NET natively compares object equality: by memory
reference with the ReferenceEquals method;
* Two DomainSignatureComparable objects which point to the same memory
location (i.e., are the same object in memory) are always equal;
* Two PersistentObject/PersistentObjectWithTypedId objects which have
the same ID are always equal;
* If two PersistentObject are compared and one of them is transient
while the other is persistent, they will always be different from a
standpoint from equality;
* Any PersistentObject properties which are not decorated with the
DomainSignature attribute will not be included in the comparison of
two PersistentObject object; consequently, ONLY the properties
decorated with the DomainSinature attribute will have an impact on
comparison.

As always, your continued feedback is most appreciated.

Billy


On Oct 4, 4:40 pm, "Simone Busoli" <simone.bus...@gmail.com> wrote:
> Billy, is the project going to be a one man show forever? Just to know if I,
> or anyone, should bother discussing changes, fixes or improvements.
>

Luis Abreu

unread,
Oct 5, 2008, 1:18:36 PM10/5/08
to sharp-arc...@googlegroups.com
Hello guys,

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

Simone Busoli

unread,
Oct 5, 2008, 1:51:59 PM10/5/08
to sharp-arc...@googlegroups.com
If two objects are persistent, doesn't it make sense to consider them equal if they have the same ID? The ID is not necessarily the ID in the DB.

Luis Abreu

unread,
Oct 5, 2008, 3:12:41 PM10/5/08
to sharp-arc...@googlegroups.com

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

Billy

unread,
Oct 5, 2008, 7:11:34 PM10/5/08
to S#arp Architecture
Assuming equal ID (and type) implies object equality is the approach
I've been using for a few years and I haven't run into an outside case
yet; not that that means that it's inappropriate in some situations,
but I have yet to run into them. My thoughts on this is that if you
legally change your name from Luis to George, that you're still the
same person that you were before but with a different property value.
Your ID in this instance would be an assigned ID being your social
security number (or possibly some auto-generated one from the DB).

Billy
> Checked by AVG -http://www.avg.com

Simone Busoli

unread,
Oct 5, 2008, 7:20:47 PM10/5/08
to sharp-arc...@googlegroups.com
This is definitely the way to go with persistent objects, at least according to DDD, where objects with an identity are called entities. Their identity is not regulated by the equality of their attributes (these are called value objects), but by a higher level concept, according to the domain in which your objects live.

Luis Abreu

unread,
Oct 6, 2008, 5:45:12 AM10/6/08
to sharp-arc...@googlegroups.com
regarding the name change, that's where the domain business signature
enters. You wouldn't promote Name to a business signature, right?

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

Luis Abreu

unread,
Oct 6, 2008, 8:42:01 AM10/6/08
to sharp-arc...@googlegroups.com
Hello guys.

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

Billy

unread,
Oct 6, 2008, 11:19:48 AM10/6/08
to S#arp Architecture
You're right in implying that the ID should never change. This is one
of my major hang-ups with assigned IDs; changing the core ID of an
entity can lead to all sorts of maladies and mass hysteria.

WRT the hash code within Equals... I've corrected the flaws in the
logic that spurred the discussion: a) comparing items with a one
letter difference at the end wasn't showing inequality and b) order of
the properties wasn't being considered. Both of these have now been
resolved while still being able to leverage the speed and efficiency
of performing bitwise comparisons.

Additionally, all tests that were submitted for consideration are
passing with the latest build. Please let me know if there's still an
outside case that is not working.

Thank you,
Billy

On Oct 6, 3:45 am, "Luis Abreu" <lab...@gmail.com> wrote:
> regarding the name change, that's where the domain business signature
> enters. You wouldn't promote Name to a business signature, right?
>
> 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....
>
>
>
> On Mon, Oct 6, 2008 at 12:20 AM, Simone Busoli <simone.bus...@gmail.com> wrote:
> > This is definitely the way to go with persistent objects, at least according
> > to DDD, where objects with an identity are called entities. Their identity
> > is not regulated by the equality of their attributes (these are called value
> > objects), but by a higher level concept, according to the domain in which
> > your objects live.
>

Luis Abreu

unread,
Oct 6, 2008, 2:39:21 PM10/6/08
to sharp-arc...@googlegroups.com
Yes, I've seen that. The problem is that there's nothing out there that says
that gethashcode will return a different value for 2 different elements. It
might happen and that is why I'm still not satisfied with using that in the
equality...

--
Luis Abreu


> -----Original Message-----
> From: sharp-arc...@googlegroups.com [mailto:sharp-
> Checked by AVG - http://www.avg.com
> Version: 8.0.173 / Virus Database: 270.7.5/1703 - Release Date: 06-10-
> 2008 09:23

Billy

unread,
Oct 7, 2008, 8:26:08 AM10/7/08
to S#arp Architecture
Hi Luis,

Could you explain a bit more about the problematic case that you're
trying to pinpoint? I didn't quite understand what you're implying.

Thanks!
Billy
> > Version: 8.0.173 / Virus Database: 270.7.5/1703 - Release Date: 06-10-
> > 2008 09:23

Luis Abreu

unread,
Oct 7, 2008, 10:35:36 AM10/7/08
to sharp-arc...@googlegroups.com
What I'm saying is that it is wrong to use the hashcode for equality. Just
think about it for a minute. A hashcode is a 32 bit integer, right? So, it's
really impossible with this "limited" number of possibilities to ensure that
each element is unique and that 2 different elements won't generate the same
hashcode. In fact, some have already proved this in the past:

http://www.interact-sw.co.uk/iangblog/2004/06/21/gethashcode

with some interesting rants on the way the docs talk about the gethashcode
method implementation :)

ok, my point is just one: I believe that it is impossible to guarantee (at
least in a 32 bit machine) that 2 different objects won't return the same
hashcode. Sure, your current implementation solved the previous bug, but can
you really say that it solves all the cases? That is why I think that
equality should only be done by comparing both instances' properties.
> Checked by AVG - http://www.avg.com

Brian Nicoloff

unread,
Oct 7, 2008, 1:04:54 PM10/7/08
to sharp-arc...@googlegroups.com
Billy,
Thanks for the acknowledgement, I'm glad it helped.
Brian

-----Original Message-----
From: sharp-arc...@googlegroups.com
[mailto:sharp-arc...@googlegroups.com] On Behalf Of Billy
Sent: Sunday, October 05, 2008 12:46 AM
To: S#arp Architecture
Subject: Re: Version 0.8.0 with MVC Preview 5, NH 2.0 and Ninject 1.0 now
available!


Simone Busoli

unread,
Oct 7, 2008, 7:46:34 PM10/7/08
to sharp-arc...@googlegroups.com
Luis, if two objects are persistent, they end up being compared according to their identifier, which is by definition unique. So those are out of this discussion.
Talking about transient ones then, do you think it's so likely that you will have about 2^32 transient objects of the same type hanging around in your application?

Dror Speiser

unread,
Nov 1, 2008, 12:59:26 PM11/1/08
to S#arp Architecture
Here's your mistake - one does not need 2^32 transient objects in
order to expect a collision, but rathar 2^16.
This is called the birthday paradox.
To put this in perspective, think of a new hit youtube video. It can
easily be accessed by 64 thousand people in a minute.
Is it really that farfetched that one can have this number of objects
in memory?

The correct solution is to iterate on the domain properties and call
Equals on each pair of values.

This will actually be better performance wise (at least most of the
time):

1. Properties of the type can be iterated only once.
2. GetHashCode is called twice for each value, but the Equals would be
called only once. Most of the time GetHashCode probably uses all the
raw bytes of the type, so 2 of them are at least as slow as one Equals
call.

In conclusion, not only is it more correct, it will be better for
performance.

Simone Busoli

unread,
Nov 3, 2008, 3:25:23 AM11/3/08
to sharp-arc...@googlegroups.com
You're right about the birthday paradox, too much time has passed from my statistics lectures. Anyway, I think that's unlikely that you'll end up with conflicts in a normal application, that's why Billy, as well as the implementation of Equals which R# provides, follow this pattern.

Luis Abreu

unread,
Nov 3, 2008, 4:16:29 AM11/3/08
to sharp-arc...@googlegroups.com
unlikely != impossible and that' why i still think that approach is wrong...

--
Regards,
Luis Abreu

Simone Busoli

unread,
Nov 3, 2008, 4:18:36 AM11/3/08
to sharp-arc...@googlegroups.com
Eheh, impossible, tell me something which is impossible in our field. 

Luis Abreu

unread,
Nov 3, 2008, 4:21:19 AM11/3/08
to sharp-arc...@googlegroups.com
> Eheh, impossible, tell me something which is impossible in our field.

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

Simone Busoli

unread,
Nov 3, 2008, 4:22:45 AM11/3/08
to sharp-arc...@googlegroups.com
I agree that this might not be the best way to go, so I think Bill will happily accept any contributions on this front.

Luis Abreu

unread,
Nov 3, 2008, 4:26:16 AM11/3/08
to sharp-arc...@googlegroups.com
yeah, I think we've discussed this in the past and I still think that
the safest approach is to compare the values instead of using their
hash values, ie, instead of calculating the hash and then using that
value to compare 2 instances, you'd just call equal over the
properties of each object...this is simple, but safer than using
hashcode...

--
Regards,
Luis Abreu

Simone Busoli

unread,
Nov 3, 2008, 4:29:27 AM11/3/08
to sharp-arc...@googlegroups.com
Agreed!

Billy

unread,
Nov 3, 2008, 8:43:56 AM11/3/08
to S#arp Architecture
This is certainly easy enough to resolve programmatically, simply by
disconnecting Equals and GetHashCode. But could someone explain to me
what the purpose of GetHashCode is if it's possible, if not likely in
large data sets, to have collisions? Additionally, the current
implementation allows GetHashCode to change if the values change,
similarly to how it can change when implemented using the NHibernate
documentation for guidance:
http://www.hibernate.org/hib_docs/nhibernate/html/persistent-classes.html#persistent-classes-equalshashcode.
But, Frederick Gheysels brought up a great find last week on my blog:
Microsoft specifically states that an object's hashcode should never
change, regardless of changes to the object; specifically, "The hash
function must return exactly the same value regardless of any changes
that are made to the object" ("http://msdn.microsoft.com/en-us/library/
system.object.gethashcode(VS.80).aspx"). But then it also states that
"if two objects of the same type represent the same value, the hash
function must return the same constant value for either object." This
would imply to me that the value of the hash is contingent on the
underlying value of the objects, if they're to be the same for two
different objects; consequently, the hashcode *should* be dependent on
the underlying values and *would* be subject to change. Indeed, the
very example Microsoft provides, concerning the "Point" object, uses
the underlying properties to create the hash and would definitely
change when the values themselves change. Is this an obvious
contradiction or am I missing something?

But regardless, it sounds like the preferred consensus would be to
split the dependency that Equals has on GetHashCode; instead, they can
both use the same underlying mechanism of using the object's property
values (the DomainSignature properties), but that the Equals should
compare each value directly rather than comparing the GetHashCode of
each object.

But my question remains, is GetHashCode inherently flawed since it can
result in collisions in large data spaces; regardless of its use or
non-use by Equals?

Billy

On Nov 3, 2:29 am, "Simone Busoli" <simone.bus...@gmail.com> wrote:
> Agreed!
>
> On Mon, Nov 3, 2008 at 10:26 AM, Luis Abreu <lab...@gmail.com> wrote:
>
> > yeah, I think we've discussed this in the past and I still think that
> > the safest approach is to compare the values instead of using their
> > hash values, ie, instead of calculating the hash and then using that
> > value to compare 2 instances,  you'd just call equal over the
> > properties of each object...this is simple, but safer than using
> > hashcode...
>
> > On Mon, Nov 3, 2008 at 9:22 AM, Simone Busoli <simone.bus...@gmail.com>

Luis Abreu

unread,
Nov 3, 2008, 9:04:11 AM11/3/08
to sharp-arc...@googlegroups.com
if i'm not mistaken, hashes will only be used by hashtables and this
is where problems might happen (what happens if you change the hash
after putting the object on a hashtable?) and why the docs mention
several rules which seem to contadict themselves (or make it
impossible to even calculate the hash correctly).

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

Frank

unread,
Nov 3, 2008, 11:41:46 AM11/3/08
to S#arp Architecture
Let's look at how Hashtables work and then perhaps this will shed
light on what hashcodes are for.

http://en.wikipedia.org/wiki/Hashtable

A hashtable is made up of a set of indexes, normally called buckets.
Each bucket is identified by a number which corresponds to the hash of
the key that will reference the value you are trying to store in the
hashtable. To find a value, the hashcode of the key is obtained (via
GetHashCode()) in order to find the correct bucket. This bucket has a
pointer off to the actual value.

So what happens when there is a hash collision? Any decent hashtable
has to deal with this situation because collisions are much more
probable than you'd think (given the birthday paradox). Collision
resolution is used to deal with this issue. There are a number of
different ways to do it, but the simplest is to use a method called
chaining. Imagine if each bucket is actually a linked-list that is
identified by a hashcode. Now when we add an entry to the hashtable
and there is a collision, just append the value to the linked-list for
that hash. This has a slight hit on performance, but it is nominal
because collisions are much rarer than non-collisions (provided your
hashcode implementation doesn't suck).

So in summary, hashcodes are allowed to be non-unique; they are never
meant to be used as a unique identifier. They are only used as a
'hint' to speed up certain data structures that make use of hashes
(like the hashtable). With this in mind, my vote is that any Equals()
implementation avoids the use of GetHashCode(). Imagine tracking down
an issue that stems from Equals() succeeding because of a hash
collision. There would be no direct evidence that this is happening,
you'd only see the side-effect (like constraints in the DB failing, or
other weird issues like what appears to be duplicate information).
Compare this debugging/fixing cost to the likely performance
difference between Equals() that calls Equals() on all members vs.
Equals() that uses GetHashCode().

-Frank

On Nov 3, 6:04 am, "Luis Abreu" <lab...@gmail.com> wrote:
> if i'm not mistaken, hashes will only be used by hashtables and this
> is where problems might happen (what happens if you change the hash
> after putting the object on a hashtable?) and why the docs mention
> several rules which seem to contadict themselves (or make it
> impossible to even calculate the hash correctly).
>
> 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...
>
>
>
> On Mon, Nov 3, 2008 at 1:43 PM, Billy <googlegro...@emccafferty.com> wrote:
>
> > This is certainly easy enough to resolve programmatically, simply by
> > disconnecting Equals and GetHashCode.  But could someone explain to me
> > what the purpose of GetHashCode is if it's possible, if not likely in
> > large data sets, to have collisions?  Additionally, the current
> > implementation allows GetHashCode to change if the values change,
> > similarly to how it can change when implemented using the NHibernate
> > documentation for guidance:
> >http://www.hibernate.org/hib_docs/nhibernate/html/persistent-classes.....

Frank

unread,
Nov 3, 2008, 12:00:19 PM11/3/08
to S#arp Architecture
I'm not sure about the contradiction myself, but just wanted to note
that the Point class isn't really an object, it's a value-type. You
have to think about what the hash is going to be used for. Once an
object is placed in a hashtable, it will be important for the hash to
remain consistent until the object is removed from the hashtable. Keep
in mind that other data structures and algorithms use the hash in a
similar role, usually as a hint for lookups. So let's run thru a quick
scenario.

1. Create object X, for whatever reason, it's hash = 42.
2. Add X to a hashtable.
3. Modify X so that it's hash (due to a less than optimal
GetHashCode() function) returns 99.
4. Do a lookup for X on the hashtable. This will fail because it will
attempt to find the value at the bucket marked 99, when in reality we
added this object under bucket 42.

So, the trick is to derive the hashcode off of something that is
consistent and won't change over the lifetime of an object. For
instance, a person's social security number is a great candidate, but
their last name is not (because it might change due to marriage/
divorce/etc).

Also, in my experience, generic hashes are neither necessary or easy
to implement. What I mean is that I typically use a hash for one kind
of indexing strategy to improve lookups in critical areas of an
application. This means there is careful consideration into the
generation of the hash as well as the data structure that will use the
hash. It is possible that you might want to index a particular object
by many different means. For instance, I might want to index people
based on their last name or their first name. In this case I'll just
access the last name and first name directly instead of trying to get
the hash for a person. You might think of the hash for a person to be
it's 'primary' index. So a generic hash that somehow uses all the
members of an object is not very useful. You want to pick members that
don't change because it is tied to that object's identity which really
means, if you change a member that also changes the core identity of
that object because GetHashCode() is derived from it, then you'll have
to go hunt down all the hashtables or other users of that hash and
update them accordingly.

-Frank

On Nov 3, 5:43 am, Billy <googlegro...@emccafferty.com> wrote:
> This is certainly easy enough to resolve programmatically, simply by
> disconnecting Equals and GetHashCode.  But could someone explain to me
> what the purpose of GetHashCode is if it's possible, if not likely in
> large data sets, to have collisions?  Additionally, the current
> implementation allows GetHashCode to change if the values change,
> similarly to how it can change when implemented using the NHibernate
> documentation for guidance:http://www.hibernate.org/hib_docs/nhibernate/html/persistent-classes.....

Dror

unread,
Nov 3, 2008, 1:48:56 PM11/3/08
to S#arp Architecture
I agree with Frank. I almost never override GetHashCode, and just use
some property of the object as a key in a Dictionary.

But, since some working objects require a good generic hash, like Set
from NH, this is what I suggest:

It's not exactly a Business Signiture, but rather a Primary Key that
we're looking for.
As the name suggests, this should be exactly like in a database and
like Frank said.
The hash should be something like the social security number or the
user's guid.
This is exactly the same as a primary key in a database, which, is
exactly the hash, so to speak, of a row.

Once you switch to thinking of primary keys, it becomes much easier to
know what really should be part of your hash creation.

Of course, this isn't to imply that GetHashCode should return a unique
number. It shouldn't because it can't.

The question of wether or not the hash should change if the values
change is tough one since it is context related.
I sugget looking into the common usage of the hash value in
NHibernate.
This will tell us more about what the nature of the hash should be.

Luis Abreu

unread,
Nov 3, 2008, 2:41:59 PM11/3/08
to sharp-arc...@googlegroups.com
I see nothing wrong with delegating to the hash of the domain signature
properties (which, by definition, don't change after having been set)...

--
Luis Abreu


> -----Original Message-----
> From: sharp-arc...@googlegroups.com [mailto:sharp-
> archit...@googlegroups.com] On Behalf Of Frank
> Sent: Monday, November 03, 2008 5:00 PM
> To: S#arp Architecture
> Subject: Re: Version 0.8.0 with MVC Preview 5, NH 2.0 and Ninject 1.0
> now available!
>
>
> No virus found in this incoming message.
> Checked by AVG - http://www.avg.com
> Version: 8.0.175 / Virus Database: 270.8.4/1750 - Release Date: 02-11-
> 2008 09:51

Billy

unread,
Nov 6, 2008, 7:32:23 PM11/6/08
to S#arp Architecture
This has been a terrific conversation; very enlightening to say the
least! Frank, I very much appreciate the time you took in explaining
GetHashCode in further detail; your insight was very assistive in
understanding the root of the problem and motivations for why it needs
to be fixed. Simone, Luis, and Dror - thank you also for your input
which was also very helpful.

To resolve this Equals/GetHashCode issue, I have checked in some
changes to the trunk which I would appreciate any and all's opinion
on; the relevant changes include:
* http://sharp-architecture.googlecode.com/svn/trunk/src/SharpArch/SharpArch.Core/DomainSignatureComparable.cs
* http://sharp-architecture.googlecode.com/svn/trunk/src/SharpArch/SharpArch.Core/PersistenceSupport/PersistentObject.cs
* http://sharp-architecture.googlecode.com/svn/trunk/src/SharpArch/SharpArch.Tests/SharpArch.Core/DomainSignatureComparableTests.cs
which has more testing and more complex testing scenarios. The
original tests were left alone.
*
http://sharp-architecture.googlecode.com/svn/trunk/src/SharpArch/SharpArch.Tests/SharpArch.Core/PersistenceSupport/PersistentObjectTests.cs
which now has the test CannotEquateObjectsWithSameIdButDifferentTypes
which was breaking when I first introduced it.

The fix has all of the original unit tests passing with a few new ones
as noted above. The change splits Equals and GetHashCode within both
DomainSignatureComparable and PersistentObject. While GetHashCode
behaves similarly to how it did before, Equals now compares the values
of each domain signature property to each other, ignoring all others.
The Equals within PersistentObject takes into account transience and
the object's ID, when available.

Thank you again for everyone's input. Any and all feedback and/or
further suggestions are most welcome! I hope these changes will put
the Equals/GetHashCode to final rest but would love your comments to
validate the changes or raise any new concerns.

Thank you,
Billy McCafferty
> > Checked by AVG -http://www.avg.com
> > Version: 8.0.175 / Virus Database: 270.8.4/1750 - Release Date: 02-11-
> > 2008 09:51- Hide quoted text -
>
> - Show quoted text -

Luis Abreu

unread,
Nov 7, 2008, 4:16:58 AM11/7/08
to sharp-arc...@googlegroups.com
I think this will solve most of the problems of the previous releases.
Good work guys!

--
Regards,
Luis Abreu

GoDSatana

unread,
Nov 17, 2008, 9:46:34 AM11/17/08
to S#arp Architecture
What do you think if not to load domain signature properties each time
we want to calculate hash code of an object, but to have them
preloaded in a
private IEnumerable<PropertyInfo> domainSignatureProperties
I think it will increase the performance...

Billy

unread,
Nov 17, 2008, 9:59:58 AM11/17/08
to S#arp Architecture
We could do it via lazy loading so it's only loaded if needed but
reusable...

private IEnumerable<PropertyInfo> GetDomainSignatureProperties() {
if (domainSignatureProperties == null) {
domainSignatureProperties = GetType().GetProperties()
.Where(p => p.GetCustomAttributes(typeof
(DomainSignatureAttribute), true).Length > 0);
}

return domainSignatureProperties;
}

private IEnumerable<PropertyInfo> domainSignatureProperties;

Billy

GoDSatana

unread,
Nov 17, 2008, 10:41:23 AM11/17/08
to S#arp Architecture
What does it mean reusable?
I think that each time we calculate the hashcode we apply this query.
Or i dont understand something?

GoDSatana

unread,
Nov 17, 2008, 10:42:53 AM11/17/08
to S#arp Architecture
What does it mean reusable?
The query will execute each time we need to calculate the hashcode.
No?

On Nov 17, 4:59 pm, Billy <googlegro...@emccafferty.com> wrote:

GoDSatana

unread,
Nov 17, 2008, 10:45:58 AM11/17/08
to S#arp Architecture
One more question. Are these two expressions the same? If they are may
be the second variant is more reusable?
private bool HasSameNonDefaultIdAs(PersistentObjectWithTypedId<IdT>
compareTo) {
return (ID != null && !ID.Equals(default(IdT))) &&
(compareTo.ID != null && !compareTo.ID.Equals
(default(IdT))) &&
ID.Equals(compareTo.ID);
}

private bool HasSameNonDefaultIdAs(PersistentObjectWithTypedId<IdT>
compareTo) {
return (!IsTransient()) &&
(!compareTo.IsTransient()) &&
ID.Equals(compareTo.ID);
}

On Nov 17, 4:59 pm, Billy <googlegro...@emccafferty.com> wrote:

Billy

unread,
Nov 17, 2008, 10:47:06 AM11/17/08
to S#arp Architecture
Sorry, reusable by the object that contains it; so it could be used by
both Equals and GetHashCode, and only get loaded once per instance.

I suppose you could load it once per object type, as it won't vary by
instances of the same type, but a clean solution for that within the
base object doesn't come quickly to mind. I'm open to suggestions...

Billy
> > > I think it will increase the performance...- Hide quoted text -

Billy

unread,
Nov 17, 2008, 10:59:48 AM11/17/08
to S#arp Architecture
Certainly simpler to read if anything...thanks for the suggestion!

Billy
Reply all
Reply to author
Forward
0 new messages