Delicious Monster
Pimp My Code, Part 5: Special Apple Sample Code Edition...
Ok, today we take a snippet of sample code from the great mothership herself, Apple, and we compare my style to theirs (or, at least, one of their programmers). Both of these routines work. You might feel one has advantages over the other, and I won't be peeved if you decide mine's not better.| Starting Position: Apple's Code from CoreData Sample Project |
| /* Change this path/code to point to your App's data store. */ - (NSString *)applicationSupportFolder { NSString *applicationSupportFolder = nil; FSRef foundRef; OSErr err = FSFindFolder(kUserDomain, kApplicationSupportFolderType, kDontCreateFolder, &foundRef); if (err != noErr) { NSRunAlertPanel(@"Alert", @"Can't find application support folder", @"Quit", nil, nil); [[NSApplication sharedApplication] terminate:self]; } else { unsigned char path[1024]; FSRefMakePath(&foundRef, path, sizeof(path)); applicationSupportFolder = [NSString stringWithUTF8String:(char *)path]; applicationSupportFolder = [applicationSupportFolder stringByAppendingPathComponent:@"ManyToManySQLTest"]; } return applicationSupportFolder; } |
| First Pass: Doll It Up a Bit |
| // Change this path/code to point to your App's data store. - (NSString *)applicationSupportFolder; { OSErr err = FSFindFolder(kUserDomain, kApplicationSupportFolderType, kDontCreateFolder, &foundRef); if (err != noErr) { NSRunAlertPanel(NSLocalizedString(@"Cannot find folder", @"error title"), NSLocalizedString(@"Your home folder appears to be missing its ""Application Support"" folder, which is required by many parts of Mac OS X. You should restore this folder from backups or create a new account and move your files over to it.", @"error text"), @"Quit", nil, nil); [[NSApplication sharedApplication] terminate:self]; } FSRef foundRef; unsigned char path[PATH_MAX]; FSRefMakePath(&foundRef, path, sizeof(path)); return [[NSString stringWithUTF8String:(char *)path] stringByAppendingPathComponent:@"ManyToManySQLTest"]; } |
I think, using the latter, it's easier to remember where each name fits into the larger picture.
Also, I change the error strings to suggest what the user should do to get out of the bind she's in. I mean, yes, it's going to be a rare condition. But if it's so rare we don't believe it'll ever happen, let's just exit the program silently if it does. We don't know that it's that rare, so we're duty-bound to have our error message not just describe the severity of the error, but also offer some suggestions on how to work around this problem.
Finally, I localized the errors, because, darn it, EVERY TIME you add an English string to a program, LOCALIZE IT. RIGHT THEN AND THERE. YOU WILL BE SORRY IF YOU WAIT UNTIL THE END. YOU WILL BREAK YOUR PROGRAM DOING LOCALIZATIONS RIGHT AS YOU ARE ABOUT TO SHIP.
Ok, so that's just a brief once-over. Can I do more? As it happens, yes. See that ugly FSRef and OSErr stuff? WTF? I never learned that in Cocoa class. Ah, because that's not part of Cocoa; it's Carbon. Snuck it right in there, didn't they. Of course, because Carbon is 22 years old, they've got to do some munging to get it to deal with modern strings and stuff, including hard-coding the maximum resolved pathlength of 1024 (I replaced it with its symbolic constant, hoping for a better day) . If only there were an easier way, using, like, strings and shit...
| Second Pass: Replace Carbon with Cocoa |
| - (NSString *)applicationSupportFolder; { NSArray *applicationSupportDirectoryPaths = NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES); if ([applicationSupportDirectoryPaths count] == 0) { NSRunAlertPanel(NSLocalizedString(@"Cannot find folder", @"error title"), NSLocalizedString(@"Your home folder appears to be missing its ""Application Support"" folder, which is required by many parts of Mac OS X. You should restore this folder from backups or create a new account for yourself and move your personal files over to it.", @"error text"), @"Quit", nil, nil); [[NSApplication sharedApplication] terminate:self]; } return [[applicationSupportDirectoryPaths objectAtIndex:0] stringByAppendingPathComponent:[[NSProcessInfo processInfo] processName]]; } |
But, hey, does NSSearchPathForDirectoriesInDomains() actually check to see if the directory it just pointed us to physically exists, or is it just saying, "Here's the name of the directory, hope that works out for you"? Because, if it's the latter, there's really no reason for us to test the status of the return, because if Mac OS X has obviated the Application Support directory altogether then we're going to need to rewrite our app to get things working; there's nothing the user can do for us.
In that case, really, we should think about our method just looking like this:
| (Optional) Third Pass: Screw Error Checking |
| - (NSString *)applicationSupportFolder; { return [[NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES) objectAtIndex:0] stringByAppendingPathComponent:[[NSProcessInfo processInfo] processName]]; } |
This version will raise an exception if Mac OS X is ever changed in an incompatible way, and the paths returned is an empty array. I highly doubt that'll happen, but, if it does, well, an exception is not the worst way to go. I mean, seriously, Apple could change any method in Mac OS X in an incompatible way at any moment, and we can't write alternatives to each call or we end up solving the halting problem.
Some people might say, "Well, I like the previous one better, it's safer, etc," but remember, each line of code you have in your project is a line you have to understand, read past, support, and maintain. That previous version had two extra English strings it added to our localization dictionary, which means you'll be paying all your localizers to translate them.
My point is, those original 15 lines of code actually had a pretty big support footprint for what they actually did. (Especially when the name of the app was hard-coded in like that.) Our goal as programmers is to solve problems (big and small) in a way that causes the smallest support burden.
Labels: code
17 Comments:
You know, the Halting Problem is only insoluable in general. There's a 50-50 chance that we CAN solve the halting problem for OS X! Them's good odds! And since we won't know until we try, I volunteer you to do it. C'mon Wil, prove to me that OS X will or will not halt: I'm considering offering a $50,000,000 + job bounty for it.*
* Offer actually void. Sucker!
** Yes, I know that the concept of a "halting problem" doesn't really apply to OS X since it has a near infinite amount of possible user input and crashing it (AKA halting) is relatively easy.
October 03, 2005 5:07 AM
A minor quibble:
The "first pass" code won't compile because the 'foundRef' variable is declared too late. A premature optimization?
October 03, 2005 5:16 AM
Of course, one long line with a lot of sub-method calls can even be worse to maintain, because you have to figure out exactly what that line is doing. I'd much rather see something like:
NSArray *searchPaths = NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory,
NSUserDomainMask, YES);
NSString *applicationSupportFolder = [searchPaths objectAtIndex:0];
return [applicationSupportFolder stringByAppendingPathComponent:[[NSProcessInfo processInfo]
processName]];
There. That's code that's easily readable at a glance, and should *ease* the support of the code despite the line count going up by two. Less lines doesn't mean easier to maintain.
October 03, 2005 5:24 AM
NSApplicationSupportDirectory was only defined in OS X 10.4. I think they probably used the Carbon crap so that it worked for pre-Tiger users.
October 03, 2005 6:31 AM
I love reading opinions about code.
Although I don't program in this language, you have helped mold my opinions about code.
Less code = better code
Declare variables just in time.
I hate having to read the entire book just to find out the name of the bar the character is in at this moment. I should only have to read the paragraph about the bar!
Consarnit!
I deal with a lot of legacy (aka. slapped together by apes) code. I have come to have very strong opinions about how code should be written.
October 03, 2005 6:42 AM
"NSApplicationSupportDirectory was only defined in OS X 10.4. I think they probably used the Carbon crap so that it worked for pre-Tiger users."
The code is from a Core Data sample project. Core Data is a Tiger-and-up technology, so while you do have a point (in that it's not supported by 10.3), in this example it doesn't need to be supported by 10.3 and Wil's code does the exact same for all users that can get the app flying in the first place.
October 03, 2005 7:00 AM
Looking at the second pass of the code, when you call -[NSApplication terminate:], if your delegate opts to delay the termination of the program, then the terminate: method will exit and continue to execute the rest of the code in the calling method. Since we know from the if statement that the array is empty, the return statement will always raise an exception when it tries to access objectAtIndex:0. So if you write something like this, you should either be absolutely certain that NSApplication will actually terminate, put the rest of the method in an else statement, or add a "return nil" after the terminate call.
October 03, 2005 9:06 AM
Cameron: Nice catch.
JJ: I'm on the fence on this one. I don't like mega-lines myself, but over the years I've come to hate defining a variable that never varies. It confuses the issue for me (why is this a variable?), and also makes the code read out-of-order.
But you're right that mega-lines are kind of messy. I wish that "const" worked a bit better in Obj-C, but it's basically just an invitation to get lots of warnings from the compiler that it's ignoring you.
Brian: You are right; it was an oversight not to also have a return statement after the call to -terminate:. I like the explicit return after a call that you know isn't returning also because it gives the compiler a better idea what's happening, so in general use there's a possibility it'll optimize your code and/or issue warnings that'll end up being important. (Eg, "Hey, this other code can't possibly be reached.")
October 03, 2005 11:23 AM
Regarding:
"But, hey, does NSSearchPathForDirectoriesInDomains() actually check to see if the directory it just pointed us to physically exists, or is it just saying, 'Here's the name of the directory, hope that works out for you'?"
In my informal tests, it appears that it just spits out the path that the folder should be at. It doesn't bother to check if the folder exists or not.
October 03, 2005 1:13 PM
You're not escaping the quotes around Application Support properly.
"Your home folder appears to be missing its ""Application Support"" folder..."
is three different strings that the compiler then immediately concatenates into one, sans quotes. Whereas
"Your home folder appears to be missing its \"Application Support\" folder..."
is one string, with proper quotes in it.
October 03, 2005 3:52 PM
Just want to say that I'm a recent Mac convert, and am enjoying reading your tidbits on Mac programming.
October 03, 2005 7:55 PM
Hmm. If it were up to me, I would create the Application Support folder if it didn't exist - at least in this case. Does the app need the folder? YES! Does the user care that the folder is missing, or does he/she just want things to work? If the app cannot create the Application Support folder, then move on to the nice dialog warnings.
October 04, 2005 8:11 AM
15 lines down to 1 line sounds good to me. :)
I'm surprised they even used Carbon in that original check.
October 05, 2005 12:03 AM
Oh my, you started pimping Apple's sample code!
It is often so badly written... Have you ever looked at Quicktime code samples? ImageCapture code samples? OUCH! Makes you wonder how these frameworks are implemented.
October 05, 2005 12:45 PM
Um yes they did slip a bit of Carbon in there.
However, the Carbon example are a little strange all together, as well.
However I did pick up on on thing I kind of like (Carbon though):
switch ( a ) {
case b:
{
// b in brackets
} break;
case c:
{
// c in brackets
} break;
default:
{
// default
} break;
}
A bit off topic, but I remember I learned this from Carbon code and still do it today.
http://brianray.chipy.org
October 05, 2005 7:54 PM
What about:
[@"~/Library/Application Support/MyApp/" stringByExpandingTildeInPath]
October 12, 2005 2:17 PM
This is what I've got in my Leopard/Xcode 3.0 CoreData non-document-based app:
-(NSString *)applicationSupportFolder {
NSArray *paths = NSSearchPathForDirectoriesInDomains(NSApplicationSupportDirectory, NSUserDomainMask, YES);
NSString *basePath = ([paths count] > 0) ? [paths objectAtIndex: 0] : NSTemporaryDirectory();
return [basePath stringByAppendingPathComponent: @"Taskspace"];
}
I can't decice if the NSTemporaryDirectory() call is a nice touch or missing the point.
May 10, 2008 1:06 AM
Post a Comment
<< Home