Objective C NSString Problem

Question

i have the below code in my progrram. i am trying to append each digit pressed to an NSString. i am getting error after the string gets appended the second time. the error is message send to deallocated object. i know this is a memory management issue. i have not released subtotal. Does anybody know why the subtotal is getting deallocated? Thanks in advance.

-(IBAction)digitPressed:(UIButton *)sender
{
       NSString *digit = [[sender titleLabel] text];
       subtotal=[subtotal stringByAppendingString:digit];
       NSLog(@"appended string is :%@",subtotal);
}

-(void)ViewDidLoad
{
  subtotal = [[NSString alloc]init];

}

Answer 1

You need to read the Cocoa Memory Management Rules. This line:

   subtotal = [subtotal stringByAppendingString:digit];

Does two things that are relevant to the memory management rules.

1. It gives you a string that you do not own since the method does not begin with new or alloc or contain copy. This means that it will go away at some point in the future without you asking. If you want to stop that, you claim ownership by retaining it.

2. It overwrites the reference to the previous value of subtotal. If you owned that object, it has now leaked. You need to release it first.

To fix the code:

First remove

subtotal = [[NSString alloc]init];

from viewDidLoad. It's pointless as it just sets subtotal to an empty string. Use this instead:

subtotal = @"";

Constant string literals never go away, aso although technically you should retain it, you can forget that without any bad consequences.

In digitPressed: you are going to need to release the old value of the string because you are about to replace it. There's two ways of doing this. Either save it to a temporary variable and release it after the append, or autorelease it before the append. Also you need to retain the new value. e.g.

NSString* temp = [subtotal];
subtotal = [subtotal stringByAppendingString: digit];
[subtotal retain];
[temp release];

or

[subtotal autorelease];
subtotal = [subtotal stringByAppendingString: digit];
[subtotal retain];

You also need to make sure subtotal is released in dealloc

Having said all that, it is far better to use properties to do as much of the memory management as possible. Declare the property in the @interface thus:

@property (copy) NSString* subtotal; // string properties should normally be copy

In the implementation, you need to synthesize it. Then viewDidLoad contains

[self setSubtotal: @""];

and your action contains

[self setSubtotal: [[self subtotal] stringByAppendingString: digit]];

You still need the release in dealloc.

Answer 2

Your viewDidLoad needs to start with a lowercase v. You should also declare a retain property for your subtotal string then use setters to assign it, so that your view controller has its own pointer to the string and it won't be deallocated by accident when it goes to the autorelease pool.

In your header file:

@property (nonatomic, retain) NSString *subtotal;

In your implementation file:

-(IBAction)digitPressed:(UIButton *)sender
{
    NSString *digit = [[sender titleLabel] text];
    self.subtotal = [subtotal stringByAppendingString:digit];
    NSLog(@"appended string is :%@",subtotal);
}

-(void)viewDidLoad
{
    self.subtotal = @"";
}

-(void)dealloc
{
    [subtotal release];
    [super dealloc];
}

But if you have a string that is constantly being modified it'd be better to use NSMutableString as Jacob Relkin mentions. It's designed for being modifiable so you don't keep creating and destroying immutable NSString objects.

Answer 3

Two things:

1 - Your viewDidLoad method is wrong, the name is viewDidLoad and you need invoke the superclass' implementation by calling [super viewDidLoad]

2 - This line is wrong:

subtotal = [NSString alloc]init];

You missed the leading [ character:

subtotal = [[NSString alloc]init];

The gist of stringByAppendingString: is that the returned NSString is autoreleased, so, you should either retain the returned NSString or make your instance variable an NSMutableString and call appendString:

@interface YourViewController : UIViewController {
   //...
   NSMutableString *subtotal;
}
@property (nonatomic, retain) NSMutableString *subtotal;
@end

@implementation YourViewController 

@synthesize subtotal;

- (void) dealloc {
  [subtotal release];
  //...
  [super dealloc];
}

- (void) viewDidLoad {
   [super viewDidLoad];
   subtotal = [[NSMutableString alloc] init];
}
-(IBAction)digitPressed:(UIButton *)sender {
   NSString *digit = [[sender titleLabel] text];
   [subtotal appendString: digit];         
   NSLog(@"appended string is :%@",subtotal);
}

//...

@end

Answer 4

You're right that you don't release subtotal, but you also never retain it either. You have a couple of memory management problems. First, you should release the old value of subtotal before assigning it a new value, and second, you should retain that new value. An example with minimal changes to what you have now:

-(IBAction)digitPressed:(UIButton *)sender
{
   NSString *digit = [[sender titleLabel] text];
   [subtotal autorelease];
   subtotal = [[subtotal stringByAppendingString:digit] retain];
   NSLog(@"appended string is :%@",subtotal);
}