views:

934

answers:

5

Hi,

I have this problem with Cocoa, I am calling a function and passing an Array to it:

Some where I call the function:

[self processLabels:labels];

And the function is as follow:

- (void)processLabels:(NSMutableArray*)labs{
    labs = [[NSMutableArray alloc] init];
    [labs addObject:@"Random"];
....
}

When debugging, I notice that no new object are being added to labels when they are added to labs. Is it because I am re-initializing labs? how could I re-initialize labels inside the function then?

I tried using byref by didn't succeed, any help is appreciated.. thanks

+3  A: 

'labs' should be initialized before you pass it to processLabels, and then it shouldn't be re-initialized.

If you can't initialize the array beforehand for whatever reason and you want processLabels to create it, you need to pass a pointer to a pointer:

[self processLabels:&labels];

and the method would change to:

- (void)processLabels:(NSMutableArray**)labs{
    *labs = [[NSMutableArray alloc] init];
    [*labs addObject:@"Random"];
....
}
Chris Karcher
+1  A: 

You need to pass in a mutable array to be able to change it (that's the definition of Mutable) - to turn an NSArray into a mutable array, use:

NSMutableArray *writableArray = [NSMutableArray arrayWithArray:oldArray];

or if you just want to make an empty mutable array:

NSMutableArray *writableArray = [NSMutableArray array];

Then pass that in.

Kendall Helmstetter Gelner
+2  A: 

generally spoken, it is preferrable to not pass mutable collections, but to provide methods which perform work on them...

in fact, in response to your code I wonder even what the purpose is of passing the 'labs' array into the function when in fact you are just overwriting it (and creating a memory leak in the process). why do that?

kent
I agree with you. If you've got a void return, why not just return the array instead?
Abizern
+1  A: 

The statement labs = [[NSMutableArray alloc] init]; makes labs to point to the new array in the scope of the method. It does not make the caller's pointer point to the new array.

If you want to change the caller's pointer, do something like this:

// The caller
NSMutableArray *labels;         // Don't initialize *labels.
[self processLabels:&labels];

...

- (void)processLabels:(NSMutableArray**)labs{
    *labs = [[NSMutableArray alloc] init];
    [*labs addObject:@"Random"];
    ...
}

That's probably a bad idea because processLabels: allocates the array but the caller is responsible for freeing it.

If you want the caller to own the array, you could write processLabels: like this:

- (void)processLabels:(NSMutableArray*)labs{
    [labs removeAllObjects];
    [labs addObject:@"Random"];
    ...
}

Or, if processLabels: is just returning a collection of labels:

- (NSMutableArray*)processLabels {
    NSMutableArray* labs = [[[NSMutableArray alloc] init] autorelease];
    [labs addObject:@"Random"];
    ...
    return labs;
}

If you want the caller to be responsible for freeing the array, remove the autorelease. In that case, convention dictates that the method name should start with alloc or new, or contain the word copy.

Will Harris
+1  A: 

Will is correct, both about correcting the existing method, and about it being a bad idea. Storing back to a by-reference parameter is certainly valid, and it's used frequently in plain C programs, but in this case it adds needless complexity. In Objective-C, the preferred idiom is to return objects using the return value first, and only store back to a pointer if the return value is already being used to return something else. Not only will this make calls to a method easier to read and write, but it conforms to standard idioms that are commonly used in other languages (such as Java and C#). It becomes quite obvious if you overwrite an array pointer by assigning to it, a potential bug that is more likely to be picked up by tools like the Clang Static Analyzer.

On a related note, you should probably also consider better method and parameter naming. (I realize this is likely a somewhat contrived example.) If you're processing "labels", and they come from some source other than the mutable array you're creating, I wouldn't name the local variable "labs" or "labels" — use a more descriptive name. Method names that are less vague about what they do can vastly improve code readability. In Objective-C, long descriptive method names are preferred. Since Xcode does code completion and the method names are less ambiguous, the end result is usually less typing.

Quinn Taylor