Is there anything wrong with this unusual NSMutableArray setter?

Question

This behaves to an outside observer mostly like @property (copy), except it has the very nice property of automatically performing any side effects that may exist in remove<Key>AtIndexes: and insert<Key>:atIndexes:. If I'm going to have such side effects, this seems really handy (DRY!)--is there anything wrong with this strange-looking accessor method?

- (void)setEdges:(NSArray *)newEdges
{
 if (newEdges != edges)
 {
 [self removeEdgesAtIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, [edges count])]];
 [edges release];
 edges = nil;
 if (newEdges)
 {
 edges = [NSMutableArray new];
 [self insertEdges:newEdges atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, [newEdges count])]];
 }
 }
}

Answer 1

You should add edges = nil; either after [edges release]; or as the else clause. Otherwise passing a nil argument will make edges a dangling pointer to a released object.

Other than that, I don't see any issues with this implementation.

Answer 2

[NSIndexSet indexSetWithIndexesInRange:] leaks itself. it creates NSIndexPath object, and leaks by 16 bytes per call.

• Nikolai Ruhe: How do you come to this conclusion?

Answer 3

It seems kind of bizarre. You could just do:

- (void)setEdges:(NSArray *)newEdges
{
 if (newEdges != edges)
 {
 [edges release];
 [edges = [newEdges mutableCopy];
 }
}

Or if you want to keep some side effects.

- (void)setEdges:(NSArray *)newEdges
{
 if (newEdges != edges)
 {
 [self removeEdgesAtIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0, [edges count])]];
 [self insertEdges:newEdges 
 atIndexes:[NSIndexSet 
 indexSetWithIndexesInRange:NSMakeRange(0, [newEdges count])]];
 }
}

That saves an release/alloc pair.

• andyvn22: Except that won't perform any side effects I may have in remove<Key>AtIndexes: and insert<Key>:atIndexes:--I'd have to repeat that code inside the accessor.
• JeremyP: @andyvn22: added an alternative.
• andyvn22: What if newEdges is nil?
• JeremyP: @andyvn22: ok, so you need a test for that eventuality.

Answer 4

It turns out that this doesn't even achieve what I set out to do: by calling those KVO-compliant methods within a KVO-compliant method, I actually generate multiple notifications and the system turns out even less helpful than a standard accessor.

I settled on this verbose but effective method:

@synthesize edges;
- (void)setEdges:(NSArray *)newEdges
{
 if (newEdges != edges)
 {
 [self willRemoveEdgesAtIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0,[edges count])]];
 [self willInsertEdges:newEdges atIndexes:[NSIndexSet indexSetWithIndexesInRange:NSMakeRange(0,[newEdges count])]];
 [edges release];
 edges = [newEdges mutableCopy];
 }
}
- (void)removeEdgesAtIndexes:(NSIndexSet*)indexSet
{
 [self willRemoveEdgesAtIndexes:indexSet];
 [edges removeObjectsAtIndexes:indexSet];
}
- (void)insertEdges:(NSArray*)newEdges atIndexes:(NSIndexSet*)indexSet
{
 [self willInsertEdges:newEdges atIndexes:indexSet];
 [edges insertObjects:newEdges atIndexes:indexSet];
}
- (void)willRemoveEdgesAtIndexes:(NSIndexSet*)indexSet
{
 //My side effects here
}
- (void)willInsertEdges:(NSArray*)newEdges atIndexes:(NSIndexSet*)indexSet
{
 //My side effects here
}

With some clever use of respondsToSelector: and performSelector:, you can even replace most of this code with a single-line preprocessor macro that automatically synthesizes all three accessors and calls your side effect methods iff you implemented them.