0

In the following code I want to traverse an NSMutableArray (self.codes) which contains NSMutableDictionary objects. If the value for the "selected" key is equal to "1" then I want to copy the NSMutableDictionary "code" object and add it to the selectedCodes array.

1  -(NSMutableArray *)getSelecedCodes{
2  
3  NSMutableArray *selectedCodes=[[NSMutableArray alloc]init];
4  
5  for (NSMutableDictionary *code in self.codes) {
6    if([code valueForKey:@"selected"]==@"1"){
7      [selectedCodes addObject:[code copy]];
8    }
9  }
10
11 return selectedCodes;
12
13 }

When I analyze the code in XCode I get a warning of the potential memory leaks. Any thoughts on what I am doing wrong?

ChrisP
  • 9,796
  • 21
  • 77
  • 121

2 Answers2

3

You should autorelease the copied object as you add it

for (NSMutableDictionary *code in self.codes) {
    if([code valueForKey:@"selected"]==@"1"){
        [selectedCodes addObject:[[code copy] autorelease]];  
    }
}

You should also use the string comparing methods as opposed to simple pointer comparison:

[code valueForKey:@"selected"]==@"1"

will never be true, instead use

[[code valueForKey:@"selected"] isEqualToString:@"1"]
jbat100
  • 16,757
  • 4
  • 45
  • 70
  • 1
    Literal strings aren't autoreleased -- they're actually immortal, and the best way to think of them is _unowned_. – jscs Nov 01 '11 at 20:39
  • @JoshCaswell thanks for pointing that out, found a few SO posts on the subject http://stackoverflow.com/questions/6069459/does-some-text-give-an-autoreleased-or-retain-1-object-back. Didn't know they just lived forever. – jbat100 Nov 01 '11 at 20:46
  • Welcome! It's interesting -- I think it's done because they're actually objects (`NSConstantString`) and it's way simpler to put them in the data segment of the binary than to fuss around with managing heap memory as if it were on the stack. – jscs Nov 01 '11 at 20:58
3

-[NSMutableArray addObject:] retains its argument. copy returns a retained value as well. So you're retaining the copied dictionary twice.

You probably want to do this:

[selectedCodes addObject:[[code copy] autorelease]];

However, depending on your specific needs, there are a couple caveats:

  1. Are you sure you want to copy the dictionary? You can just add it to the array:

    [selectedCodes addObject:code];

    Note that both selectedCodes and self.codes will contain pointers to the same dictionary, so changes in one copy will be reflected in the other as well.

  2. You might want to create a mutable copy of the dictionary. copy returns an immutable object:

    [selectedCodes addObject:[[code mutableCopy] autorelease]];

mipadi
  • 398,885
  • 90
  • 523
  • 479