Change the value of NSString, am I doing it right?

Question

I have created a class as follows

@interface sampleClass: UIViewController
{
    NSString *currentLocation;
}
@property (nonatomic, copy) NSString *currentLocation;

So, whenever the current GPS changes, a function will be called as follows:

-(void)newLocationUpdate:(NSString *)text {
  NSString *temp = [[NSString alloc] initWithString:text];
  currentGPS = temp;
  [temp release];
}

I am wondering if I am doing it right? Thanks.

Answer 1

If you have your currentLocation/currentGPS setter implemented properly (with @synthesize or a manual implementation), then you're doing too much work. If you have a property declared with the copy flag, as you do, then the setter method itself will do the copy that you're doing by hand. All you would need is this line:

[self setCurrentGPS:text];

Or, if you prefer the property syntax:

self.currentGPS = text;

That will automatically call the copy method, which is basically a more efficient way of doing what you're doing the long way with [[NSString alloc] initWithString:text].

Answer 2

Nope, you're doing it wrong.

Your class sampleClass has an ivar named currentLocation and a property named currentLocation. In your newLocationUpdate method you are setting a variable named currentGPS to the string value passed to that method. As written, currentGPS has no relationship with either the variable currentLocation or the property currentLocation.

Also, your use of init and release in your newLocationUpdate method looks like you may have a fundamental misunderstanding of how memory management in Obj-C works. You should definitely read Apple's guidelines on memory management.

http://developer.apple.com/library/mac/#documentation/Cocoa/Conceptual/MemoryMgmt/MemoryMgmt.html

This is important stuff! Good luck.

Answer 3

Here:

@interface sampleClass: UIViewController
{
NSString *currentLocation;
}
 -(void)newLocationUpdate:(NSString *)text;

.m

-(void)newLocationUpdate:(NSString *)text {
  currentLocation = text; 
}

That's the way I would do it. Is it correct? Probably not. Will it work? Yes.

Answer 4

Ignoring the currentLocation/currentGPS confusion — no, that's still not quite right.

You don't show the the setter for currentLocation. I'll assume it's a @synthesized property. If you just write currentLocation = something, you're not invoking the property setter; you're just setting the instance variable. This means that after you release the object in the very next line, your instance variable is probably pointing to a deallocated object.

The correct way to write it (again, assuming you have a synthesized accessor) would be:

-(void)newLocationUpdate:(NSString *)text {
  self.currentLocation = text;
}

This invokes the property accessor, which copies the object for you.

If for some reason you needed to access the instance variable directly (like if this were the setter method for currentLocation), you would write:

-(void)newLocationUpdate:(NSString *)text {
  [currentLocation release];
  currentLocation = [temp copy];
}