Objective C memory problem

Question

Hi all,

I have 2 scenarios and I want to know which way is correct and better?

---

-(Employee*) getCustomEmployee{

  Employee *emp = [[Employee alloc]init]; // no autorelease here
  //do something with emp

  return emp;
}

-(void) process{

   Employee *emp = [self getCustomEmployee];
   //do something with emp

   [emp release]; //release emp object here

}

---

-(Employee*) getCustomEmployee{

  Employee *emp = [[[Employee alloc]init]autorelease]; //NOTE: autorelease here 
 //do something with emp

  return emp;
}

-(void) process{

   NSAutoreleasePool *pool = [[NSAutoReleasePool allc]init];

   Employee *emp = [self getCustomEmployee];
   //do something with emp

   [pool release];

}

Answer 1

Generally the second one -- in Objective-C, you normally return autoreleased objects from a method unless it's an init or copy method.

Answer 2

The autoreleased way is more typical, but you can do it either way, with one caveat:

• If you are returning an object that the caller needs to release, you should name your routine createCustomEmployee instead of getCustomEmployee, to follow the Cocoa naming conventions. (Routines with create or alloc in their name return an object that must be released).

Answer 3

The 2nd is better practice but still incorrect. You are coding a getter which should not create memory. Instead it should return a reference to already created memory. It would be better to rename it as a factory method, "customEmployee" and use auto release as that is the naming convention encouraged by Apple and found in other API calls. You could also have a createCustomEmployee that doesn't autorelease. The naming convention is what's important here.

Answer 4

I don't think the AutoreleasePool is required in the second one. I'm pretty sure there's a default one. So, unless your process method is very intensive, the pool is not necessary.

I would say the autorelease is the way to go.

Imagine you have to call the getCustomEmployee methods from different places in you code. Each time you'll call this method, you'll have to remember to release the Employee.

Answer 5

If you are eager to follow the Cocoa's convention, you should have an explicit answer to your question in this article

Answer 6

The second is better.

From the Memory Management Programming Guide:

• You own any object you create.
• You “create” an object using a method whose name begins with “alloc” or “new” or contains “copy” (for example, alloc, newObject, or mutableCopy).

Since your getter method doesn't match any of those criteria it should not make caller the owner of the returned object implicitly. Callers should explicitly invoke retain on the returned value if they want to own it.

That being said, you don't need the NSAutoreleasePool in your code because, by default, the run loop already has a pool. With that, the following should suffice:

- (Employee*) getCustomEmployee {
   Employee *emp = [[[Employee alloc] init] autorelease];
   // do something with emp
   return emp;
}

- (void) process {
  Employee *emp = [self getCustomEmployee];
  // do something with emp
}

Other answerers say that you should not be allocating new objects in getters. I disagree with that. Creating objects on demand is a good practice in many cases. Specifically when you have resource intensive objects that may never need to be allocated. It's perfectly acceptable to allocate new objects in a getter so long as the ownership rules are adhered to.

In this case, if the employee can be shared between multiple callers, then you might want to consider lazy-initialization. The following code assumes that you have an instance variable (a ivar in objective-c lingo) called _employee.

- (Employee*) getCustomEmployee {
   @synchronized(self)
      if (!_employee) {
         _employee = [[Employee alloc] init];
      }
   }
   return _employee;
}

- (void) dealloc {
   [_employee release];
}

- (void) process {
  Employee *emp = [self getCustomEmployee];
  // do something with emp
}

In this case the ownership stays with the object that created it, therefore it must release the memory at some point in the future. The natural place for releasing owned objects is when the object itself is released. When that happens the dealloc method will be called, so that's where we can do our cleanup.

Answer 7

Leaving aside the fact that you shouldn't prefix these methods with 'get' according to Apple naming conventions, the second version is the more correct one since it conforms to Apple's memory management rules.

You could make the first example better by prefixing the method name with 'new' e.g. newCustomEmployee. However, use of new is sort of deprecated.