Converting NSArray to NSSet, custom class instances transfer inconsistently

Question

Ran into a interesting little problem. I was writing a method to filter an array to the unique objects:

- (NSArray*)distinctObjectsByAddress {
    NSSet* uniqueSet = [NSSet setWithArray:self];
    NSArray* retArray = [uniqueSet allObjects];

    return retArray;
}

and wrote a unit test to check:

- (void)testDistinctObjectsByAddress5 {
    Person* adam1 = [[Person alloc] initWithFirstName:@"adam" lastName:@"adam" andParent:nil];
    Person* adam2 = [[Person alloc] initWithFirstName:@"adam" lastName:@"adam" andParent:nil];

    testPersonArray = [NSArray arrayWithObjects:adam1,adam2, nil];

    NSArray* checkArray = [testPersonArray distinctObjectsByAddress];

    STAssertEquals([checkArray count], [testPersonArray count], @"Array %@ counts should match %@ %@",checkArray,adam1,adam2);
}

Pretty simple. The interesting part is that about 80-90% of the time the test passes and every so often it fails because the distinctObjectsByAddress method only returns one object. I've been able to trace it to the [NSSet setWithArray:self] call but I've also been able to verify that the two person objects are two different objects (at least they have different address). I'm assuming that setWithArray: is just doing a basic address compare but I don't understand why it is sometimes producing two objects like it should and sometimes producing only one.

Something I just tried was changing adam2 so that the first and last name were not exactly the same as adam1. This seems to fix the error. Does this point to some sort of compiler optimization when the objects are logically the same?

Adytum

I would guess that the problem is with Person and how it implements the hash and equality methods that NSSet uses. – Viborg

It'll be using isEqual: as defined by the NSObject protocol to compare objects; have you implemented it or hash on Person? – Posture

In addition to the hash problem, you're not explicitly testing adam1 and adam2 for nil. If one occasionally fails to init, that would explain the test failure. – Culinary

Answer 1

I'm assuming that setWithArray is just doing a basic address compare

That's incorrect. NSSet uses the -isEqual: and -hash methods on the objects that are added to it. It depends on how those are implemented in Person or its superclasses.

If [person1 isEqual:person2] then you would expect the set to contain one object. If not, then the set should contain two objects.

My guess is that Person does not follow the rules in its -isEqual: and -hash methods. Most likely, the two objects are equal, but their hashes are not equal like they should be. (Except for the 10-20% of the time that you're getting lucky.)

Does this point to some sort of compiler optimization when the objects are logically the same?

No, there is no compiler optimization that would merge the two objects into one.

Dowable

Answer 2

Most likely you did not implement hash for Person, and sometimes the identical Person object hashes into two different buckets.

Viborg

hmm, I did override isEqual but no hash so that is probably the problem, I have since taken the NSSet code out as it seemed a bit 'hacky' and wrote the code to compare the addresses myself. But I'm still really interested as to why this was happening. This hash thing is probably it, I'll check and accept if it is. – Adytum

@Adytum per the docs "If two objects are equal, they must have the same hash value." ( developer.apple.com/library/mac/#documentation/Cocoa/Reference/…: ); failure to implement NSObject correctly could validly result in any behaviour. – Posture