Search code examples
objective-cnsmutableset

*[NSMutableSet addObject: X]* adds object X even though *[Y isEqual:X]* returns TRUE for an object Y already in the Set


I have a Objective-C class called FactorHelper whose definition is below. It has a property called factors which is an NSMutableArray of NSNumbers. I have a custom isEqual: method in this class that returns true if the factors property in the two FactorHelper objects have the same numbers (even if the numbers are in a different order).

I tried to test by creating two FactorHelper objects one with 10,5,2 and the other with 10,2,5. Then I created a NSMutableSet, added firstObject and then second object. I was expecting second object to be not added, but I see that it is added. When I step through the code I find that isEqual is being called by addObject and is returning TRUE. What am I doing wrong?

UPDATE

Changing the [NSMutableSet new] to [NSMutableSet alloc] init] makes things work as expected.

Also, changing all the TRUE, FALSE is isEqual to YES, NO makes it behave correctly (even if I keep it as [NSMutableSet new]).

I have no idea what is going on. Can someone please shed some light?!

Class definition

@interface FactorHelper: NSObject
 @property NSMutableArray <NSNumber *> *factors;
 -(BOOL) isEqual:(FactorHelper *)other;
 -(instancetype) initWithFactors:(NSMutableArray *)factors;
 -(NSString *) description;
@end

@implementation FactorHelper

- (instancetype) initWithFactors:(NSMutableArray *)factors
{
    self = [super init];

    if (self) {
        _factors = factors;
    }

    return self;
}

-(BOOL) isEqual:(FactorHelper *)other
{
    if ([self.factors count] != [other.factors count])
    {
        return FALSE;

    }
    else
    {
        NSMutableDictionary <NSNumber *, NSNumber *> *myHashTable = [[NSMutableDictionary alloc] init];
        for (NSNumber *nextNumber in self.factors) {
            if(myHashTable[nextNumber] == nil)
            {
                myHashTable[nextNumber] = @(1);
            }
            else
            {
                myHashTable[nextNumber] = @([myHashTable[nextNumber] integerValue]+1);
            }
        }

        for (NSNumber *nextNumber in other.factors)
        {
            if(myHashTable[nextNumber] == nil)
            {
                return FALSE;
            }
            else
            {
                myHashTable[nextNumber] = @([myHashTable[nextNumber] integerValue] - 1);

                if ([myHashTable[nextNumber] integerValue] == 0) {
                    [myHashTable removeObjectForKey:nextNumber];
                }
            }
        }

        if ([[myHashTable allKeys] count] == 0)
        {
            return TRUE;
        }
        else
        {
            return FALSE;
        }

    }
}
@end

Unit test code

NSMutableSet *testSet = [NSMutableSet new];
FactorHelper *fact1 = [[FactorHelper alloc] initWithFactors:[@[@(10),@(5),@(2)] mutableCopy]];
FactorHelper *fact2 = [[FactorHelper alloc] initWithFactors:[@[@(10),@(2),@(5)] mutableCopy]];
[testSet addObject:fact1];
[testSet addObject:fact2];
NSLog(@"Are factors 1 and 2 the same: %d",[fact1 isEqual:fact2]);

Solution

  • NSMutableSet is a hash value based set. You need to override hash method for its element types consistent with isEqual:.

    In your case, something like this:

    - (NSUInteger)hash {
        NSCountedSet *factorCounts = [[NSCountedSet alloc] initWithArray:self.factors];
        return [@"FactorHelper" hash] + [factorCounts hash];
    }
    

    I'm not sure how you checked if I see that it is added, but this makes your FactorHelper work with NSMutableSet.

    By the way, your isEqual: can be implemented a little bit shorter utilizing NSCountedSet.

    -(BOOL) isEqual:(FactorHelper *)other {
        NSCountedSet *myFactorCounts = [[NSCountedSet alloc] initWithArray:self.factors];
        NSCountedSet *otherFactorCounts = [[NSCountedSet alloc] initWithArray:other.factors];
        return [myFactorCounts isEqual:otherFactorCounts];
    }
    

    This shows clearer consistency with the hash above.