March 25, 2006

Pimp My Code, Part 9: Beginner code.

Today a fellow blogger asked me to pimp his post. Since it's only two lines, I figure I can take a break from my busy schedule of, uh, drinking and stuff, and help a brother out.
So I did the Challenge problem in Chapter 4 of Cocoa Programming for Mac OS X, Second Edition. I've come up with two different "solutions".

Solution 1: "Screw retain counts"
- (IBAction) reportCharacterCount: (id)sender
{

NSString *inputString = [inputField stringValue];
[outputField setStringValue:[NSString stringWithFormat:@"%@ has %d letters.",
inputString,[inputString length]]];
}

Solution 2: "I'm a good boy"
- (IBAction) reportCharacterCount: (id)sender
{

NSString *inputString = [[NSString alloc] initWithString:[inputField stringValue]];
[outputField setStringValue:[NSString stringWithFormat:@"%@ has %d letters.",
inputString,[inputString length]]];
[inputString release];
}

For pedagogical reasons, could someone tell me what the difference is between the two? And if possible, which one is better?

(As you can probably guess, my very first solution consisted of version 1 with a release, which brought me to the debugger in a hurry.)
The short answer is the difference between the two is that the second one wastes time and memory to no good effect. There are several problems with the second one: for example, if you really wanted a immutable copy of a string, you should just use [[inputField stringValue] copy] and not -initWithString:, because the latter always allocates a new string, whereas the former will just return the same object with an increased retain count if the original string was already immutable. Now that's fast!

But, in fact, there's no point in doing this copy of the inputField's string, for two reasons. First off, when you call -setStringValue: on outputField it's really the field's job to make sure it holds on to a immutable copy of the string you've passed in, so it's going to call -copy or do something similar itself. (It's true there were bugs in early versions of NeXTstep where sometimes mutable strings would be retained or returned instead of immutable ones, but those are mostly ironed out now.)

Secondly, and more importantly, you're not actually passing this string directly to your output NSTextField, you are generating a new, autoreleased string in your +stringWithFormat: call, which has the inputField's stringValue as a sub-string. Now, leaving aside the actual implementation details of +stringWithFormat:, it's a given that it will somehow keep an immutable copy of any strings you pass into it. Otherwise, honestly, nothing in this damn system would work.

Less code is better if it's functionally the same, and the second implementation is absolutely no safer in any way. Even if you were, say, messing with multiple threads at some point, so the value of inputField could change during your action method, both implementations would be failures, so there's really no conceivable situation in which the second implementation is better.

Also, what's up with that blank line at the beginning of your methods? Seriously, that isn't helping anyone.

Finally, I should point out both implementations are really non-optimal in the post-10.3 world: what you should really do is bind the 'value' inputField to a new instance variable in your controller in Interface Builder (say, NSString *inputString;), and then bind outputField's 'value' to your controller with the path of, say, 'outputString', then write the following:

Solution 3: Bindings
+ (void)initialize;
{
[self setKeys:[NSArray arrayWithObject:@"inputString"]
triggerChangeNotificationsForDependentKey:@"outputString"];
}

- (NSString *)outputString;
{
return [NSString stringWithFormat:@"%@ has %d letters.",
inputString, [inputString length]]];
}

Admittedly, this isn't really less code, and in fact it change the semantics of your app: eg, it doesn't require you to send the action to populate the outputField. But bindings are generally the way you should code these days; if you find yourself using target/action, or otherwise manually pushing or pulling values to or from controls, think hard about using bindings instead.

Labels:

15 Comments:

Blogger Drew Hamlin said...

Wil, good post. I am a little (perhaps pleasantly) surprised at your suggestion to use bindings as this example as I have to admit it didn't even occur to me. How often do you advocate doing this? It seems like you're suggesting here pretty never use target/action anymore. Is that true? Are there exceptions? Is that what Apple advocates too? Thanks.

March 25, 2006 7:18 PM

 
Anonymous Matt Ronge said...

Bindings in most cases can replace target/action. Now, that's not to say there is never a need for target/action, it's just bindings can be used for 90% of the cases out there.

March 25, 2006 10:35 PM

 
Anonymous Dominik Wagner said...

It always astounds me how hard it is for people to really grasp the retain/release/autorelease concept of memory management. I often see people do very weird things just because they think it is cleaner. More often then not they end up doing things like in this post, which really doesn't help at all.

March 26, 2006 3:40 AM

 
Blogger Chris Foresman said...

That last bit seems to have too many closing brackets, or am I just crazy? (Note: These conditions are not necessarily mutually exclusive.)

March 26, 2006 8:15 AM

 
Blogger William Henderson said...

Maybe the real question is, would you have used KVO? If I were using KVO, it would seem obvious (to me, anyway) to use bindings in this example...

March 27, 2006 6:01 AM

 
Anonymous Tim said...

Seems like a good time to plug the Stepwise articles. From Very simple rules for memory management in Cocoa:

2. Objects created using convenience constructors (e.g. NSString's stringWithString) are considered autoreleased.

See also: Memory Management with Cocoa/WebObjects and
Hold Me, Use Me, Free Me

March 27, 2006 10:03 PM

 
Anonymous Tim said...

Err, nevermind me. That quote doesn't apply directly here, though the situation with values obtained from accessor methods is very similar.

March 27, 2006 10:15 PM

 
Anonymous Anonymous said...

Wow, Wil..

I been programming a long time in windows... for that matter unix, and never thought of that elegant solution....

Time to go read the docs on bindings more, and get more creative.

March 28, 2006 4:03 AM

 
Blogger Abhi Beckert said...

I love bindings, they allow simple solutions for complicated problems.

March 29, 2006 5:26 AM

 
Blogger Richie said...

I too am going through Cocoa Programming for Mac OS X by Hillegass. While I agree that bindings is the way to go, it's not covered until a latter chapter (chapter 6 I believe). The first few chapters focuses on target/action.

April 03, 2006 5:31 AM

 
Anonymous Jess said...

I've recently rediscovered the world of macs... and your brilliant delicious library, which makes me very badly want to go buy a mac and a camera and start scanning everything within reach. Nice work.

April 11, 2006 9:20 PM

 
Blogger Fabian Spillner said...

Yeah, I always said, the bindings can be more than be said.

May 04, 2006 4:45 AM

 
Anonymous Stephen said...

Enough lurking already... This really took me off guard; using bindings for even the most basic of variables and actions. I set this up as shown and it runs great, so again I'm doubly surprised that something so short can be so mind-bending.

I just started gnawing on bindings and core data this week and I haven't until now seen the method setKeys:triggerChangeNotificationsForDependentKey: ... could you please distinguish between this and bind:toObject:withKeyPath:options:, which I have been using extensively? And here I thought maybe I was doing *something* right.

And if I may: could you expand on how bindings fits into the contemporary Cocoa programmer's toolkit? What I mean is, if your solution to this exercise is really the optimal, then perhaps Cocoa newbies working on that first currency converter ought to be taught how to do it with bindings, from right there onwards. What's more, now when I am going through the docs I can choose to implement a datasource, a delegate, bindings, possibly other tools, and now bindings supposedly replaces target-action as well (to an extent).

Is this a case of the shiny new hammer? Are we better off with so many options for how to solve a problem? If Bindings are the new orange, I would like to see some other whole styles get deprecated, because I for one have made the mistake before of using old techniques and only later finding out that 10.4 extincted the poor sap (NSURLClient protocol, anyone?)

peace

September 14, 2006 10:21 PM

 
Anonymous Mike said...

I've actually just finished this challenge and I've come up with something a bit different.

- (IBAction)CountCharecters:(id)sender
{
[tbOutput setStringValue:[NSString stringWithFormat:@"%@ has %d charecters",[tbInput stringValue], [[tbInput stringValue] length]]];
}

I do not understand the need to create a new instance of the input as you've done in both solution 1 and 2, when you've said less code is better.

June 18, 2007 9:35 AM

 
Blogger Wil Shipley said...

Mike:

I think you're missing the point -- solutions 1 and 2 weren't mine, they were submitted by the poster. Solution 3 is mine.

June 18, 2007 2:04 PM

 

Post a Comment

<< Home