manual memory-management in object-c

Question

I have a question about manual memory-management in object-c when I'm reading this guide.

For question to be more clearer, I paste the code confused me below:

// CarStore.m
- (void)setInventory:(NSMutableArray *)newInventory {
    if (_inventory == newInventory) {
        return;
    }
    NSMutableArray *oldValue = _inventory;
    _inventory = [newInventory retain];
    [oldValue release];
}

I think above code does a duplicate operation:

// CarStore.m
- (void)setInventory:(NSMutableArray *)newInventory {
    // if we can ensure '_inventory' is different with 'newInventory'
    if (_inventory == newInventory) {
        return;
    }

    // we can release '_inventory' firstly and safely
    [_inventory release];
    _inventory = [newInventory retain];
}

also

// CarStore.m
- (void)setInventory:(NSMutableArray *)newInventory {
    // if we don't check the equal between '_inventory' and 'newInventory' firstly
    // then we need temp '_inventory', since '_inventory' and 'newInventory' maybe the same object
    NSMutableArray *oldValue = _inventory;
    _inventory = [newInventory retain];
    [oldValue release];
}

I'm not sure if there are some bugs in my thought or not, so please give me a hand.

Answer 1

You're mostly correct and either of your versions is common.

There is one subtle potential problem with the middle snippet, though. It's possible that newInventory is different from _inventory and yet releasing _inventory will still result in newInventory being deallocated. That's because it may be that the only remaining strong reference to newInventory is _inventory itself, if newInventory is contained, directly or indirectly, in _inventory.

There's yet a third form that you could use that avoids the need for a temporary variable:

- (void)setInventory:(NSMutableArray *)newInventory {
    [newInventory retain];
    [_inventory release];
    _inventory = newInventory;
}

Finally, there may be reasons why you might want to include a check for equality even if it's not strictly necessary for correct memory management. For example, although Apple works hard to make -retain and -release fast, they are not free. So, it might still be an efficiency improvement to skip them if the value isn't really changing.

Also, you may want to put other work in your setter, like marking a view as needing display or invalidating related cached values. You may wish to avoid doing so if the value isn't really changing.