CODEWITHSUNDEEP
iphoneobjective-cpropertiesretainautorelease
Jamie
Jamie

Reputation: 5112

Should I retain an autoreleased object when assigned to a retained property?

In the getter for one of my retained properties, I allocate and assign an NSArray to my model object("models"). I then sort that NSArray using "sortedArrayUsingSelector:" method. According to the docs, this returns an autoreleased NSArray back. I then reassign that to my "models" array.

At first, I never retained this autoreleased sorted array and found that when I popped this viewController off the stack, my app would crash, saying that I was trying to decrement the reference count on a deallocated object or something of the sort. I then added a retain as you can see in the code below, and all is ok.

My question is, is this correct that I should have to retain the autoreleased object, even though I am retaining this property in its declaration and releasing it in my dealloc?

- (NSArray *)models {

    if (!models) {
        models = [[NSArray alloc] initWithArray:[self.modelDictionary allKeys]];
        models = [[models sortedArrayUsingSelector:@selector(compare:)] retain];
    }
    return models;

}

Upvotes: 1

Views: 1258

Answers (3)

bbum
bbum

Reputation: 162712

In general, if you want an autoreleased object to stick around, you must retain it.

There are a couple of issues with this code:

- (NSArray *)models {

    if (!models) {
        models = [[NSArray alloc] initWithArray:[self.modelDictionary allKeys]];
        models = [[models sortedArrayUsingSelector:@selector(compare:)] retain];
    }
    return models;
}

  • you are leaking the array that is alloc/initd with the second models= line of code

  • you said "retaining this property in its declaration, but in the above code you are assigning to the instance variable directly, bypassing the setter method that may have been generated by @synthesize

  • it is a getter method that changes the state of the variable being got... while this kind of lazy initialization can be attractive, it must be done with extreme care. Core Data goes to great lengths -- through quite a bit of internal complexity -- to allow for lazy population of state in this fashion. The above code would also mean that Key-Value Observers of the models property would not see the change when the getter is executed. Yet, if you call willChangeValueForKey:/didChangeValueForKey: manually from the getter, you'll end up sending a change notification from within the getter, quite likely causing something to consume the value before whoever called the getter first retrieves the value. Sound confusing? It is -- that may or may not cause a problem. It is, in my experience, likely to cause a problem some day.

Some suggestions:

  • use Core Data, if you can. A truly huge amount of engineering work has been invested in making Core Data very efficient and very well integrated with the system.

  • separate getter/setters from the rest of your logic. More likely than not, there is a logical point in your app where you'll need to "get" the models. When you do, tell the object to fault 'em in through some other method then get the models. It may seem like a pain, but construction of the object graph is very much a specialized task vs. querying that graph for information (or use Core Data, which has a solution for this).

Upvotes: 6

D.C.
D.C.

Reputation: 15588

Unless I"m missing something, it looks like you are just setting the result to one of your instance variables. I think you should be using your property instead. 

self.models =  [NSArray arrayWithArray:[self.modelDictionary allKeys]];

Without the self. part, you are not using the setter method, and thus never retaining anything.

Upvotes: 1

Jamie
Jamie

Reputation: 5112

Thank you! I understand what you mean. I think I got confused and thought that calling self.models in my getter would create an infinite loop. I see now that it is just if I call the getter from within the getter not the setter from within the getter. I changed my code to this: 

- (NSArray *)models {
    if (!models) {
        self.models = [[NSArray alloc] initWithArray:[self.modelDictionary allKeys]];
        self.models = [models sortedArrayUsingSelector:@selector(compare:)];
    }
    return models;
}

everything compiles and works fine...does this look like a more correct solution? Thanks for your time.

Upvotes: 0

Related Questions