1

I have a class that adopts the MKAnnotation protocol. Along with the two "title and "subtitle" methods to implement, I wanted to add two NSStrings, one to represent each line in a typical US address. For example:

addressLine1 = 123 street
addressline2 = northridge, ca 91326

My subtitle method currently looks like:

- (NSString *)subtitle {
    NSMutableString * ret = [NSMutableString stringWithCapacity:kDefaultStringCapacity];
    if (streetAddress) {
        [ret appendString:streetAddress];
        addressLine1 = [[NSString alloc] initWithFormat:streetAddress];
    }
    ... more code

When would I release addressLine1? Because it is a property that I declared with (nonatomic, retain), I already released it in my dealloc method. Or am I to use a class method or autorelease the string? Thanks.

Deepak Danduprolu
  • 44,595
  • 12
  • 101
  • 105
Crystal
  • 28,460
  • 62
  • 219
  • 393
  • 1
    Why do you use a format with no additional arguments? With no additional arguments, it's not actually doing anything but copying the format string as-is, and the presence of any % format specifiers in streetAddress will likely crash your app. What are you trying to accomplish there? – Sherm Pendley Jun 09 '11 at 06:13

2 Answers2

3

If you autorelease address1, you will lose ownership on the object and without any other owners, it will get deallocated. You would need to autorelease it if you were doing,

self.address1 = [[NSString alloc] initWithString:streetAddress];

which is wrong as you would've taken ownership twice and relinquished it only once in the dealloc method. Right way would've been,

self.address1 = [[[NSString alloc] initWithString:streetAddress] autorelease];

The direct assignment above works only if it were to be assigned a value once. If it is to be assigned again, you would lose the reference to the earlier reference and your application will leak. So it would be good process to use the property accessors here which would ensure that the older values are deallocated.

Another thing with strings is that you would copy them as you wouldn't want them to mutate after assignment so the property should be declared as @property (nonatomic, copy) rather than what it is now.

Deepak Danduprolu
  • 44,595
  • 12
  • 101
  • 105
  • By using `copy` instead of `retain`, there’s no need whatsoever to use `+alloc/-init…` — the OP can use `self.address1 = streetAddress` instead (assuming he’s using `+alloc/-init` to avoid referencing the same string in both `streetAddress` and `self.address`). –  Jun 09 '11 at 06:20
  • Note that Crystal used -initWithFormat: - although in a rather pointless way, as I said in my comment above. With the property declared as copy, and assuming the use of a format string in the example is just a typo, `self.address1 = streetAddress` would be far more straightforward. – Sherm Pendley Jun 09 '11 at 06:20
  • Yeah, I was just coming back to add that. There was no need for the `alloc-init`. It would've made sense if there was a `initWithFormat:` but in that case, `stringWithFormat:` would be the right way to go. – Deepak Danduprolu Jun 09 '11 at 06:26
2

If your property is (nonatomic, retain), then you're going to leak the addressLine1 resource if you don't explicitly release it. I'd release it as soon as you're done with it. The property should then be released in your dealloc method as you are currently doing.

Somewhat unrelated to the question, but still related, is that anytime you have an object that implements the NSCopying protocol, such as NSString in this case, you should use copy instead of retain. Here's a SO question that provides some great information.

Community
  • 1
  • 1
csano
  • 13,266
  • 2
  • 28
  • 45
  • 2
    That *would* be a leak if he'd used dot-syntax to set the property. Doing so would have gone through the accessor, which - as you said - would have added an extra retain in addition to the one that's implicit in `+alloc`. With only one release in `-dealloc`, that would indeed be a leak. But he *didn't* use dot-syntax - he assigned directly to the `addressLine1` ivar, bypassing the setter and its retain. So the one release in `-dealloc` is all that's required, or even allowed. – Sherm Pendley Jun 09 '11 at 06:03
  • @Sherm Unless `-subtitle` is invoked more than once (and enters that `if`), in which case the OP needs to release the previous object. –  Jun 09 '11 at 06:05
  • True, true. I'd use dot syntax, and a "convenience" constructor for the string that doesn't imply ownership, as in `self.addressLine1 = [NSString stringWithFormat:streetAddress]`. I'm also wondering what the point of using a format is, when there are no arguments following it... – Sherm Pendley Jun 09 '11 at 06:09
  • @Sherm/@Bavarious You're right. I assumed that when OP mentioned the property that she was setting the property using dot-syntax. Thanks for pointing out my mistake. – csano Jun 09 '11 at 06:26