views:

190

answers:

3

I have an iPhone application which has some methods to gather information from the web and which then creates an object, which holds this information as properties (lets say I'll get objects of type x). Since I might need those kind of objects from various points within the application, I put the method to create one instance of the object into a single implementation file and called that file "GetDetails.m" (+ h). In this method I exclusively have class methods*, (amongst those my method to create object x) which create the Object and fill its properties with the information that the method gathers from the web.

+ (ObjX *)getObjectX:(NSString *key) withParameters:(NSArray *)parameters;

Within this method my ObjX is filled with information...

ObjX *objectX = [[ObjX alloc] init];
(...)
objectX.name = gatheredName;  // etc
(...)
return objectX;

So my class method is invoked at several points from within the app (btw. from within a separate thread):

NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
ObjX *myObject;
myObject = [GetDetails getObjectX:@"09384f9a" withParameters:nil];
[self performSelectorOnMainThread:@selector(doStuffWithNewObject:) withObject:myObject waitUntilDone:NO];
[pool release];

I can't get rid of the feeling that this approach is not optimal. The Xcode analyzer also tells me that there might be a potential leak on return objectX.

Maybe someone could point me to the right direction and how I should do the splitting up of functionalities in general.

*It also doesn't have an init and dealloc method, no ivars, no instance methods...


Edit

Okay, obviously most see no release for the object, so I guess I was not clear with that – the Object will be released later after it's assigned to a property of a view. Still, I see that it's practice to return an autoreleased object. Unfortunately, my App crashes when I do that. Maybe it's because of I am running that whole processing within a separate thread / autorelease pool? I will test some configurations and will let you know how it went.

+1  A: 

One thing that sticks out for me is that each time you call your class method you do:

ObjX *objectX = [[ObjX alloc] init];

Here you are creating a new instance of some object. However you code example does not show anywhere that you are releasing it. Remember alloc gives the object a ref count of one. Further, by calling your method getXXX you are going against memory management conventions. this would imply that you are retrieving a ref to some object that you do not own. Since this is not the case, you should at the very least call your method newXXX or createXXX to imply that that caller is responsible for the memory allocated. A better approach would be to autorelease it and retain it by the caller.

darren
I will go with the autorelease, you're right, it's obviously best practice. Thanks. I edited my original post.
ff10
+1  A: 

The analyzer will be giving you grief because you do not autorelease your objects when you return them. The usual method would look as follows:

ObjX *objectX = [[ObjX alloc] init];
...
return [objectX autorelease];

Now, as far as implementation goes, this sounds like a place to use categories. I would do something like this:

GetDetails_ObjX.h

@interface ObjX (GetDetails)
+ (ObjX *)objXWithIdentifier:(NSString *)ident parameters:(NSArray *)params;
@end

GetDetails_ObjX.m

@implementation ObjX (GetDetails)
+ (ObjX *)objXWithIdentifier:(NSString *)ident parameters:(NSArray *)params
{
    ObjX *objectX = [[ObjX alloc] init];
    ...
    return [objectX autorelease];
}
@end

GetDetails.h

#import "GetDetails_ObjX.h"
#import "GetDetails_ObjY.h"
...

This setup could be used as follows:

#import "GetDetails.h"
...
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
ObjX *myObject;
myObject = [ObjX objectXWithIdentifier:@"09384f9a" parameters:nil];
[self performSelectorOnMainThread:@selector(doStuffWithNewObject:)
                       withObject:myObject
                    waitUntilDone:NO];
[pool release];

A couple of notes, here:

  1. I used the method signature objXWithIdentifier... because I'm not sure what the @"09384f9a" string actually is. If it is an address, you would use objXWithAddress.... If it is a reference number, you would use objXWithReferenceNumber..., etc.

  2. I had to look this up to be sure. The performSelectorOnMainThread... method retains its argument, so there is no concern about the object being released before the method gets called on the main thread.

e.James
+1  A: 

From the Memory Management Programming Guide for Cocoa: 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). If your method does not begin with any of these prefixes, you should return an autoreleased object. Then, the caller of the method can retain the object if it needs to be kept around longer.

Mike
Thanks, I changed my original post.
ff10