views:

325

answers:

4

I am new to Objective-C and I am a little curious about how I should be managing the memory for the local NSString variables shown below and the associated instance variables inside the class object. The code I have works fine, but just curious as to best practice.

Edited to include full code, nothing special, like I say I am just curious if in this context I should be doing alloc/release on the NSString objects.

// MAIN ------------------------------------------------------------------- **
#import <Foundation/Foundation.h>
#import "PlanetClass.h";

int main (int argc, const char * argv[]) {

    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];

    NSString *planet_01_Geek;
    NSString *planet_02_Geek;

    // Create planets
    PlanetClass *newPlanet_01 = [[PlanetClass alloc] init];
    [newPlanet_01 setGeekName:@"StarWars"];
    PlanetClass *newPlanet_02 = [[PlanetClass alloc] init];
    [newPlanet_02 setGeekName:@"Dune"];

    // Query a planet
    planet_01_Geek = [newPlanet_01 geekName];
    planet_02_Geek = [newPlanet_02 geekName];

    // Print details
    NSLog(@"Planet Geek    = %@", planet_01_Geek);
    NSLog(@"Planet Geek    = %@", planet_02_Geek);

    // Clean up
    [newPlanet_01 release];
    [newPlanet_02 release];
    [pool drain];
    return 0;
}

..

// CLASS HEADER ----------------------------------------------------------- **
#import <Foundation/Foundation.h>

@interface PlanetClass : NSObject {
NSString *geekName;
}

- (NSString*) geekName;
- (void) setGeekName:(NSString*)gName;
@end
// ------------------------------------------------------------------------ **

..

// CLASS BODY ------------------------------------------------------------- **
#import "PlanetClass.h"

@implementation PlanetClass

- (NSString*)geekName {
    return geekName;
}
- (void)setGeekName:(NSString*)gName {
    geekName = gName;
}
@end
// ------------------------------------------------------------------------ **
+2  A: 

That would depend on how you have the property for "geekName" set up. I would assume that it just returns a reference to the existing member in the class rather than creating a copy? If that's the case, you shouldn't need to worry about releasing anything in the code you have there.

You should only need to worry about releasing the "geekName" member in the dealloc() for the class it is in.

Eric Petroelje
I see, my class interface is as below, so what your saying is I should really have a dealloc method for the class that does a [geekName release]; I was thinking it might get freed when I released the planetClass object (i.e. [newPlanet_01 release];)@interface PlanetClass : NSObject { float gravity; float mass; char *name; NSString *geekName; }Cheers gary
fuzzygoat
@fuzzygoat - depends on how you allocated that string originally. If you used alloc() or something similar, you need to release it. If you assigned it from a static "quoted string" then it doesn't need to be released.
Eric Petroelje
A: 

Variables are free. Local variables are allocated for you by the compiler; instance variables are allocated by the runtime as part of the instance.

The objects whose pointers you put in the variables are not free; you may need to release them. Whether you need to release an object or not is determined by the rules.

Peter Hosey
+6  A: 

Read the memory management rules. 9 simple paragraphs that explain everything you need to know.

Because geekName does not begins with “alloc” or “new” or contains “copy”, it should be returning a string you do not “own”. As such, you do not need to (and indeed, must not) release it, and you also must not store a reference to it. You may return it from the method you are in, in which case your method name also should not begins with “alloc” or “new” or contains “copy”.

If you wish to keep it around, you must take ownership of it by calling retain, or because its an NSString, better is copy. This might be automatic if you assign it to a copy/retain property.

In the code you have now posted, you have made an error in your setter. Your setter should be taking a copy of the input parameter, something like:

- (void)setGeekName:(NSString*)gName {
    if ( geekName != gName ) {
        [geekName release];
        geekName = [gName copy];
}

You then also need a dealloc routines which releases geekName:

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

Alternatively, you can use Objective C properties. Instead of your interface showing:

- (NSString*) geekName;
- (void) setGeekName:(NSString*)gName;

Use a property:

@property (nonatomic, copy) NSString* geekName;

And instead of your implementation of the setters and getters, let the system synthesize them for you:

@synthesize geekName;

You still need the dealloc method to free geekName.

Peter N Lewis
A: 

Depending on how you are using the two local variables you might need some more memory management.

The way your code reads, you are setting the local variables to the pointers returned by the two class objects. If you've written the newPlanet* classes correctly, then the string objects should be released when the classes are released. If your two local variables then try to use the pointers, you'll have problems as the objects are no longer ther

Two possible corrections:

1. Retain the strings explicitly

As the local variables are being assigned directly, you should have really retained the objects explicitly:

planet_01_Geek = [[newPlanet_01 geekName] copy];
planet_02_Geek = [[newPlanet_02 geekName] copy];

I am specifying copy here because that's the preferred way of keeping hold of objects that might by mutable, otherwise if the original changes, the local variable will also change.

2. Use properties (preferred)

This would be my preferred method: the retain, copy, or assign for the instance variables are then handled by the class.

Declare the properties correctly, i.e:

@property (nonatomic, copy) NSString *planet_01_Geek;
@property (nonatomic, copy) NSString *planet_02_Geek;

Use @synthesize in the implementation.

Then use the property syntax to allocate the variables.

self.planet_01_Geek = [newPlanet_01 geekName];
self.planet_02_Geek = [newPlanet_02 geekName];

This way the correct memory management rules will apply on assignment, and the synthesized accessor methods will also take care of releasing any object that is currently assigned to the local variables.

Edit - A note since further class details shown

In your setGeekName: method you are leaking memory. When you allocate a new pointer to the local variable, you aren't sending a release to the object that used to be in there. A better way to do it (using retain rather than copy to keep it simple:

- (void)setGeekName:(NSString *)gName {
    [gName retain]; // Hold on to the object that is passed in.
    [geekname release]; // Let go of your current object.
    geekName = gName; // Now allocate the new object.
}
Abizern
Hi Abizern, I just trimmed down the code (its only early learning stuff) and posted it in its entirety above. I am just reading your reply right now, many thanks.
fuzzygoat
I have only just started out (last week to be exact), I am aware of @synthesize and properties, but have not gotten as far as looking yet, I think they are in chapter 9, I am still on 3 :)
fuzzygoat
That's okay; keep this in mind for when you get to that part.
Abizern
I certainly will do, many thanks.
fuzzygoat
Hi, hmm I was under the impression that I was setting an existing item that was created when I created the planet object from the class.Am I right in thinking that what is happening is that I am not modifying an existing NSString object, but rather creating a new one, unless the old object has the same value as the new one in which case we just use the old one.Also when you first run the code, is there a NSString object in existance (maybe thats created when the object was created) I am just a little curious in that situation what I would be releasing.
fuzzygoat
In Objective-C you can send messages to `nil` objects safely.
Abizern