1

Beginner question here:

I'm trying to understand the some basic memory management. if I'm overriding a setter method, does the following adequately handle the memory management of the incoming string?

- (void)setMyString:(NSString *)string
{
    if (_myString != string){
        [string retain];
        [_myString release];
        _myString = string;
    }
}

My logic here is that as soon as we enter into that if statement, i want to take ownership of the incoming string, hence the retain. Next I release the _myString object. Then i set the _myString object to the string object. Here is the source of my confusion: do I need to retain the myString object at this point? or do i already have ownership over it as a result of setting it equal to the string object?

Thanks!

Sean Danzeiser
  • 9,141
  • 12
  • 52
  • 90

3 Answers3

4

What you've written is correct except for the final return _myString statement.

When you call retain on string, you are incrementing that instance's reference count by one. Assinging the value of string to _myString does not change the actual instance (now pointed to by both string and _myString), so a second retain is not necessary, and would be incorrect.

All that said, what you've got is sort of redundant. The reason for the if (_myString != string) check is that if your setter is called with the same object you already have, you don't want to release that object before you've had a chance to retain it. The program will crash in that situation, because you release it, it gets deallocated, but you keep a reference to it and continue using it (sending it messages). Since the object is the same, you can avoid this problem by simply doing nothing if the argument to the function is the same as the current value of the instance variable. However, retaining the argument before you release the instance variable is another way of accomplishing the exact same thing.

So, you can do either this:

- (void)setMyString:(NSString *)string
{
    if (_myString != string) {
        [_myString release];
        _myString = [string retain];
    }
}

or this:

- (void)setMyString:(NSString *)string
{
    [string retain];
    [_myString release];
    _myString = string;
}

I tend to favor the first approach because it is (very slightly) faster if the value is the same, and is a little simpler when written out. But really, it's up to personal preference, and the form in your original question is fine too.

Andrew Madsen
  • 21,309
  • 5
  • 56
  • 97
  • First example is wrong. If you do use ARC, you can't use `retain`. If you don't use ARC, you're missing `[_myString release]` -> memory leak. This example will not work with ARC and will leak without ARC. – zrzka Oct 11 '12 at 19:42
  • second one code may crash if are using setter and _myString == string – NeverBe Oct 11 '12 at 19:44
  • Thanks for catching that Robert. Just a typo. Note that none of this code is at all relevant if you're using ARC. You can just do `_myString = string` in that case, and ARC will take care of the proper, safe memory management calls. – Andrew Madsen Oct 11 '12 at 19:44
  • yes, a release on _myString before the set. And the second one is sufficient, it will not crash. but it will retain and release when its not needed. small enough overhead that its not really an issue. – The Lazy Coder Oct 11 '12 at 19:45
  • @NeverBe, no, as explained in my answer, retaining `string` before releasing `_myString` ensures that if they are the same, the instance is not released (the net reference count is unchanged). – Andrew Madsen Oct 11 '12 at 19:45
1

Don't use retain and release anymore. Just use ARC, it will save you a lot of troubles. You don't have to deal with memory leak, writing a lot of retain/release, etc.

And it will work with iOS 4, and since the current iOS version is 6, you're good to use ARC now than last year when ARC was new.

Coolant
  • 448
  • 6
  • 13
0

there is also a bit shorter form:

- (void)setFoo:(NSString *)aFoo {
  [_foo autorelease];
  _foo = [aFoo copy];
}

Also, a hint from the google objective-c styleguide:

Never just retain the string. This avoids the caller changing it under you without your knowledge. Don't assume that because you're accepting an NSString that it's not actually an NSMutableString.

Bastian
  • 10,403
  • 1
  • 31
  • 40
  • This is not a good practice. -autorelease is for those situations where you're returning an object which you don't need to keep, but the caller might. Don't use it when can just use -release. – NSResponder Oct 27 '12 at 10:28
  • this is part of the official google objective c styleguide http://google-styleguide.googlecode.com/svn/trunk/objcguide.xml – Bastian Oct 27 '12 at 16:00
  • Google is not an authority on objective-C coding style. Using -autorelease in this situation is a waste. – NSResponder Oct 28 '12 at 06:18
  • Ok ... so show me a document of an authority that states that it is bad practice :) – Bastian Oct 28 '12 at 22:14
  • No authority is required: Think it through. What is the cost of sending an immediate message versus queuing up and sending a delayed message? – NSResponder Oct 29 '12 at 13:11
  • It's insignificant. Or do you have values that proof something else ? – Bastian Oct 29 '12 at 13:58