2
\$\begingroup\$

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])]];
        }
    }
}
\$\endgroup\$

4 Answers 4

2
\$\begingroup\$

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.

\$\endgroup\$
1
\$\begingroup\$

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

\$\endgroup\$
1
  • \$\begingroup\$ How do you come to this conclusion? \$\endgroup\$ May 2, 2012 at 16:48
0
\$\begingroup\$

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.

\$\endgroup\$
4
  • \$\begingroup\$ 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. \$\endgroup\$
    – andyvn22
    Apr 17, 2012 at 4:41
  • \$\begingroup\$ @andyvn22: added an alternative. \$\endgroup\$
    – JeremyP
    Apr 17, 2012 at 9:10
  • \$\begingroup\$ What if newEdges is nil? \$\endgroup\$
    – andyvn22
    Apr 17, 2012 at 13:42
  • \$\begingroup\$ @andyvn22: ok, so you need a test for that eventuality. \$\endgroup\$
    – JeremyP
    Apr 17, 2012 at 14:07
0
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.