Become a MacRumors Supporter for $50/year with no ads, ability to filter front page stories, and private forums.

cblackburn

macrumors regular
Original poster
Jul 5, 2005
158
0
London, UK
Hey everyone,

I have the following code in my program:-

Code:
- (int)browseForImage:(NSString *)path
{
	int result = -1;
	NSOpenPanel * panel = [[NSOpenPanel alloc] init];
	
	[panel setAllowsMultipleSelection:false];
	result = [panel runModalForDirectory:NSHomeDirectory() file:nil types:[NSImage imageFileTypes]];
	
	if(result == NSOKButton)
	{
		path = [[[[panel filenames] objectAtIndex:0] copy] retain];
	}
	
	return result;
}

When I call the function however the NSString* path is invalid when I return to the calling function?

What is going on here?

Chris
 

kainjow

Moderator emeritus
Jun 15, 2000
7,958
7
A few changes:

Code:
- (int)browseForImage:(NSString **)path
{
	int result = -1;
	NSOpenPanel * panel = [NSOpenPanel openPanel];
	
	[panel setAllowsMultipleSelection:NO];
	result = [panel runModalForDirectory:NSHomeDirectory() file:nil types:[NSImage imageFileTypes]];
	
	if(result == NSOKButton)
	{
		*path = [panel filename];
	}
	
	return result;
}

Call it like so:
Code:
NSString *myPath = nil;
[self browseForImage:&myPath];

Summary of changes:

  1. Use openPanel method to return an NSOpenPanel, instead of alloc/init. You can use alloc/init, but it's unnecessary, and you'd have to release it later, or else you'd leak memory
  2. I changed "false" to "NO", which is more consistent with Cocoa programming
  3. NSOpenPanel is a subclass of NSSavePanel, which has a "filename" method. I used that instead. Also, a retain after a copy method is unnecessary. "copy" retains the new object automatically.
  4. If you want to change a variable that is passed in as a parameter, you must pass in a pointer to it. So I made that change. However, a more natural way to do it would be to return the value:

Code:
- (NSString *)browseForImage
{
	int result = -1;
	NSOpenPanel * panel = [NSOpenPanel openPanel];
	
	[panel setAllowsMultipleSelection:NO];
	result = [panel runModalForDirectory:NSHomeDirectory() file:nil types:[NSImage imageFileTypes]];
	
	if(result == NSOKButton)
	{
		return [panel filename];
	}
	
	return nil;
}

Now call it like:
Code:
NSString *myPath = [self browseForImage];
if (myPath)
	// do something with myPath

I'd suggest reading up on pointers and memory management some more.
 

savar

macrumors 68000
Jun 6, 2003
1,950
0
District of Columbia
kainjow nailed it, but I want to reiterate one of his points:

The natural way to do what you're trying to do is to return a pointer, rather than stuffing the pointer into a function parameter.

In vanilla C, there are a lot of libraries where they prefer side-effect programming (i.e. call methods which only return error codes, the actual work they do is by manipulating memory directly) over functional programming (call methods which return the data you need and don't modify main memory directly).

The reason behind this is most likely two-fold. First, Unix is built on a model where sub-processes return a status code to the calling process when they complete. This is an easy way to a calling process to see if the child process succeeded or not, particularly when writing shell scripts.

Second, C doesn't have exception handling built-in. So to provide a consistent interface for checking and handling exceptional conditions, developers make all routines return an error code (0 for success, other values mapped to specia meanings), and return any actual data in pointers that were passed by the calling code.

In 2006, Java, Cocoa, and .NET have exception handling built-in, so there's no need to pass around return codes, and we can go back to proper functional programming (which fits into object programming much more cleanly as well).
 

cblackburn

macrumors regular
Original poster
Jul 5, 2005
158
0
London, UK
In vanilla C, there are a lot of libraries where they prefer side-effect programming (i.e. call methods which only return error codes, the actual work they do is by manipulating memory directly) over functional programming (call methods which return the data you need and don't modify main memory directly).

Can you tell I come from a background of C/C++ Unuix Programming :-D.

Sorry, I'm not used to doing functions like that. In fact my old lecturer used to castrate us if we wrote functions that returned data. "You only ever return an error code, manipulate the data through a pointer!"

Thanks guys

Chris
 

cblackburn

macrumors regular
Original poster
Jul 5, 2005
158
0
London, UK
[*]NSOpenPanel is a subclass of NSSavePanel, which has a "filename" method. I used that instead. Also, a retain after a copy method is unnecessary. "copy" retains the new object automatically.

I found this on the Apple website:-

If multiple selections aren’t allowed, the array contains a single name. The filenames method is preferable over NSSavePanel’s filename to get the name or names of files and directories that the user has selected.

That is why I acessed the array rather than the method you used :)

Thanks for your help

Chris
 

savar

macrumors 68000
Jun 6, 2003
1,950
0
District of Columbia
Can you tell I come from a background of C/C++ Unuix Programming :-D.

Sorry, I'm not used to doing functions like that. In fact my old lecturer used to castrate us if we wrote functions that returned data. "You only ever return an error code, manipulate the data through a pointer!"

Thanks guys

Chris

Hah, well that explains it. The tricky thing about Cocoa is that they use pointers *everywhere*...passing opaque references to Obj-C objects results in a compiler error.

Which means if you want to stuff a result into a parameter in Obj-C, you need to pass a pointer to a pointer in almost every situtation.

The style you're used to is still very much present in OS X, though. All of the Carbon APIs are written in C and so they use that same approach. It can be a little madening to constantly switch back and forth in your mind when you're writing a Cocoa app that calls Carbon APIs.
 
Register on MacRumors! This sidebar will go away, and you'll see fewer ads.