Memory Management Question

Question

i thought i getting the hang of Cocoa memory management, but apperently i have a lot more to learn.

Take a look at this class i wrote:

Word.h

#import <UIKit/UIKit.h>


@interface Word : NSObject {
    NSString *word;
    NSMutableArray *wordArray;
}

@property (nonatomic ,retain) NSString *word;
@property (nonatomic ,retain) NSMutableArray *wordArray;

@end

Word.m

#import "Word.h"


@implementation Word

@synthesize word, wordArray;

- (void)setWord:(NSString *)aWord {
    NSMutableArray *newValue = [[NSMutableArray alloc] init];
    self.wordArray = newValue;
    [newValue release];

    for (int i = 0 ;i < [aWord length] ;i++) {
        NSString *character = [[NSString alloc] init];
        character = [NSString stringWithFormat:@"%C",[aWord characterAtIndex:i]];
        [wordArray addObject:character];
        //[character release];
    }
}

- (void)dealloc {
    [word release];
    [wordArray release];
    [super dealloc];
}

@end

As you can see i'm filling an array when i'm setting the string. I'm dong this in a for loop by pulling out the char of the string and putting them in a new allocated string. Then i put that string in the array and then i have to release the string. And it's here i do it wrong, because i'm releasing the value i'm putting in to the array. when later i'm trying to use the array, its gone.

How should i go about this? it feels wrong to make "character" a property and releasing it in dealloc because its just temporary.

Answer 1

First of all, you don't need to assign a new NSString to character, as you immediately overwrite it by another object. Remember, it's a pointer, so when you assign to it, you are not changing the object that it points to, only pointing it towards another object. So the object that you allocated and originally assigned to character is leaked on the next line, when we lose the pointer to it.

Now, stringWithFormat returns an autoreleased string, which character then points to. This object shouldn't be released by you, but that's what happens in the commented out line. So that loop would be better as:

for (int i = 0 ;i < [aWord length] ;i++) {
        NSString* character = [NSString stringWithFormat:@"%C",[aWord characterAtIndex:i]];
        [wordArray addObject:character];
}

Answer 2

Apart from the memory management concern, do you know that you have overridden the setter function for the word property?

When you @synthesize your word property, the setter and getter methods are generated behind the scenes so there are already functions for - (void)setWord:(NSString *)aWord; and - (NSString *)word; You can read about this in the Objective-C primer or the Programming Language documentation.

Since you have created a the (void)setWord:(NSString *)aWord; function, this overrides the generated setter. Since you don't actually set the word variable, your property is "broken".

As an example, perhaps rewrite it as:

#import "Word.h"

@implementation Word

@synthesize word, wordArray;

- (void)setWord:(NSString *)aWord {
    // set the word iVar
    if (aWord != word) {
        [aWord retain]; // I prefer [aWord copy];
        [word release];
        word = aWord;
    }

    NSMutableArray *newValue = [[NSMutableArray alloc] init];

    for (int i = 0 ;i < [aWord length] ;i++) {
        NSString *character = [NSString stringWithFormat:@"%C",[aWord characterAtIndex:i]];
        [wordArray addObject:character];

    }
    // Now that you have created the newArray, set it to the property
    self.wordArray = newValue;
    [newValue release];
}

- (void)dealloc {
    [word release];
    [wordArray release];
    [super dealloc];
}

@end

Edit

Why did I write that I preferred [aWord copy] over [aWord retain] ? Because I would have declared the properties as copy rather than retain.

Mutability can be a pain. Let us suppose that instead of passing an NSString* to aWord somehow an NSMutableString* is passed instead. This is possible because NSMutableString is a subclass of NSString. Let us also suppose that another part of the program changed the value of this string (it is mutable after all). Now, we have changed a property of the Word class externally. This breaks encapsulation.

It makes sense to use @property (copy)… for the collection classes.

You can read a good answer here.