Memory management issues when passing items between arrays

I'm attempting to move a group of strings between three different places by calling this function getNextRandomItem. This works the first time I call it, but then I get an access error the second time. Can you explain what I'm doing wrong?

Ne开发者_StackOverflow社区w Items:binNew (NSMutable Array) -> Current Item (NS String) -> Old Items:binOld (NSMutable Array)

-(NSString *) getNextRandomItem {
    if (binNew.count > 0){
        if (currentItem) {
            [binUsed addObject:currentItem];
        }

        int r = floor(arc4random() % binNew.count);
        currentItem = [binNew objectAtIndex:r];
        [binNew removeObjectAtIndex:r];
        return currentItem;
    }
    return @"No more items!";
}

---

You have to retain r when it is removed from the array. Now you remove it and set its pointer to currentItem. The retain count should become 0 so that probably will cause the error. You have two options.

1) Add a property in your interface:

@property (retain) NSString *currentItem;

and add this to your implementation

@synthesize currentItem;

then use

self.currentItem = ...

instead of

currentItem = ...

2) Use correct memory management. See altered code:

-(NSString *) getNextRandomItem {
    if (binNew.count > 0){
        if (currentItem) {
            [binUsed addObject:currentItem];
            [currentItem release]; //See here
            currentItem = nil; //See here
        }

        int r = floor(arc4random() % binNew.count);
        currentItem = [[binNew objectAtIndex:r] retain]; //See here
        [binNew removeObjectAtIndex:r];
        return currentItem;
    } else {
       [currentItem release];
       currentItem = nil;
    }
    return @"No more items!";
}

Note currently you never set your currentItem to nil. You should do that.

---

This line

currentItem = [binNew objectAtIndex:r];

should be

currentItem = [[[binNew objectAtIndex:r] retain] autorelease];

When you remove object from container with [binNew removeObjectAtIndex:r], it receives release message and without retain-ing it, you get an invalid pointer (because it points to released object).

---

-(NSString *) getNextRandomItem {
    if (binNew.count > 0){
        if (currentItem) {
            [binUsed addObject: currentItem];
            [currentItem release]; //here
            currentItem = nil;//not really necessary
        }

        int r = floor(arc4random() % binNew.count);
        currentItem = [[binNew objectAtIndex:r] retain]; //and here
        [binNew removeObjectAtIndex:r];
        return currentItem;
    }
    return @"No more items!";
}