Rixstep
 About | ACP | Buy | Industry Watch | Learning Curve | News | Products | Search | Substack
Home » Learning Curve » Developers Workshop

Simple Toolbar

Looking at sample code.


Get It

Try It

'Simple Toolbar' is a sample app provided with the ADC tools. It's located here.

/Developer/Examples/AppKit/SimpleToolbar

'Simple Toolbar' shows you how to construct an NSToolbar. It also shows you how to add an NSSearchField to a toolbar. Search fields aren't the same as toolbar glyphs so the sample code is put to good use.

When completed the app'll look like this.

The first thing you'll notice about this program is that there are bugs in the code. The old style '~/Documents' icon in the leftmost position on the toolbar is for the 'save' action: it only wants to be enabled if there's actually something to save. When starting a new file this equates to having done nothing at all; but as time goes on the two diverge.

Yet you'll find that unless you completely empty the undo buffer the glyph won't go disabled again.

Or turn the matter on its head: why should you not be able to save an empty file? This is the kind of mistake first term Visual Basic programmers make.

Either way there's a bug. Or a design flaw.

The search field however behaves correctly: it's not about to let you search for something if there's no content to search.

But to address these bugs - and to better understand how the code works - it's necessary to go into the code. Which is why it's provided. Yet doing so leads to any number of distractions - for reasons which gradually become obvious.

It should also be pointed out that there is nothing extraordinary about this sample app. It's fairly typical of the output of corporations like Apple and Microsoft.

SaveDocumentItemImage.tiff

The image used for the 'save' action on the toolbar is included in the project. It's called SaveDocumentItemImage.tiff. It's 1918 bytes. It can be compressed further. To 1906 bytes. Not a lot to be sure. But still and all. Having the image at 1918 bytes provides no benefit whatsoever.

MyDocument.h

The main header file MyDocument.h needs cleaning. This is how it looks 'out of the box'.

//
//  MyDocument.h
//
//  Copyright (c) 2001-2002, Apple. All rights reserved.
//

#import <Cocoa/Cocoa.h>

@interface MyDocument : NSDocument {
@private
    IBOutlet NSTextView     *documentTextView;
    IBOutlet NSTextField    *searchFieldOutlet;
    IBOutlet NSToolbarItem    *activeSearchItem;
    IBOutlet NSWindow        *documentWindow;
    IBOutlet NSView        *contentView;

    NSData            *dataFromFile;
}
@end

We know this is Apple code; and we know Apple let people use this code; so let's dispense with the formalities for now. Further: IBOutlet resolves to nothing.

#ifndef IBOutlet
#define IBOutlet
#endif

So let's remove it. Further: @private is default so let's remove that too. Finally: let's put everything in alphabetical order so we can find it again.

#import <Cocoa/Cocoa.h>

@interface MyDocument : NSDocument {
    NSData        *dataFromFile;
    NSTextField   *searchFieldOutlet;
    NSTextView    *documentTextView;
    NSToolbarItem *activeSearchItem;
    NSView        *contentView;
    NSWindow      *documentWindow;
}
@end

Or if we want to get really fancy we declare all these outlets as ids.

#import <Cocoa/Cocoa.h>

@interface MyDocument : NSDocument {
    id activeSearchItem, contentView, dataFromFile, documentTextView, documentWindow, searchFieldOutlet;
}
@end

Better still. But there's one remaining thing: keeping definitions in the header file and keeping the method file exclusively for code. There's namely something buried in the method file we want to move in here instead.

#import <Cocoa/Cocoa.h>

@interface MyDocument : NSDocument {
    id activeSearchItem, contentView, dataFromFile, documentTextView, documentWindow, searchFieldOutlet;
}
@end

@interface ValidatedViewToolbarItem : NSToolbarItem @end

Now we're ready. But even now before we go any further we need to 'clean' out the source code file MyDocument.m.

MyDocument.m

The first thing we see when we open MyDocument.m is that there are a lot of different formatting things going on. We want to get this code tidy so we can read it and learn from it. Others who might want to use the code need to do the same.

The code was formatted with tabs set to eight spaces. This might have been alright twenty five years ago but it's not now. No matter what anyone tells you: get rid of the spaces and substitute them for tabs. Your code is going to indent a lot more than IBM punch cards did. You want to be able to set the tab value to something that works for you.

JavaScript programmers like to indent two spaces at a time. Fine - set the tabs to two spaces and tab instead.

And so forth. First steps are to change all sequences of eight spaces into tabs; then four spaces into tabs; then clean away all the extraneous white space. And we almost might have something we can work with. [Knock on wood.]

NeXT's code editor was bad twenty years ago at leaving white space bombs all through source code files; the editor used in Xcode hasn't changed - in this sense it's the same editor.

Before the above three steps: 17648 bytes on disk. After: 16850 bytes. We just saved 798 bytes and we haven't built the program yet. But who's counting?

main.m

main.m: this 100 byte file (it starts as 110 bytes but 10 bytes are wasted) is wasting space. Let's put it in MyDocument.m. It'll speed up builds too - only one file to compile. And with certain compiler configurations you'll get leaner code too.

MyDocument.m - Not So Slight Return

It's time to look at MyDocument.m. We're almost ready. There's but one more thing we need to do - put the methods in alphabetical order. There's no way the original programmer or posterity are going to find anything in the file as is. Once we've done that we start looking at the individual methods.

static NSString*

But right at the top we see three of these.

static NSString* MyDocToolbarIdentifier         = @"My Document Toolbar Identifier";
static NSString* SaveDocToolbarItemIdentifier   = @"Save Document Item Identifier";
static NSString* SearchDocToolbarItemIdentifier = @"Search Document Item Identifier";

Some people forget. Or perhaps never learned. NSString* is not a data type - NSString is. C has no 'pointer' data type. Beyond that it's a question of storage. Our dear author is going to use the above as constants. Except they're not. [The static qualifier does nothing - even with main.m all it would do was save a few bytes object code.]

The three pointers to type NSString however are variables - whether the author likes it or not. And as such they demand storage. Above and beyond the string constants they'll point to.

It's important to distinguish between the one and the other here. The three string constants will take their own storage in the executable and have their own symbolic addresses throughout the build; the three pointers to NSString will also take storage. Anywhere from 12 to 96 bytes depending on how data aligns. You don't need this and you don't want it.

#define MyDocToolbarIdentifier            @"My Document Toolbar Identifier"
#define SaveDocToolbarItemIdentifier      @"Save Document Item Identifier"
#define SearchDocToolbarItemIdentifier    @"Search Document Item Identifier"

Now there's no way anybody else's code can muck with the values either. So you're safer and leaner and meaner both.

MyDocument (Private)

There are some category methods declared in the methods file that immediately need to be moved to the header file.

@interface MyDocument (Private)
- (void)loadTextViewWithInitialData:(NSData *)data;
- (void)setupToolbar;
- (NSRange)rangeOfEntireDocument;
@end

And while you're at it: put them in alphabetical order.

#import <Cocoa/Cocoa.h>

@interface MyDocument : NSDocument {
    id activeSearchItem, contentView, dataFromFile, documentTextView, documentWindow, searchFieldOutlet;
}
@end

@interface MyDocument (Private)
-(void)loadTextViewWithInitialData:(NSData *)data;
-(NSRange)rangeOfEntireDocument;
-(void)setupToolbar;
@end

@interface ValidatedViewToolbarItem : NSToolbarItem @end

And now for a look at the methods themselves.

dataRepresentationOfType:

Here's the original. Messy. A lot of work for nothing.

- (NSData *)dataRepresentationOfType:(NSString *)aType {
    NSData *dataRepresentation = nil;
    if ([aType isEqual: @"My Document Type"]) {
    dataRepresentation = [documentTextView RTFDFromRange: [self rangeOfEntireDocument]];
    }
    return dataRepresentation;
}

He's assigning a value to a stack variable twice. And he doesn't even need that variable. What he really meant was this.

-(NSData *)dataRepresentationOfType:(NSString *)aType {
    return ([aType isEqual:@"My Document Type"])?
        [documentTextView RTFDFromRange:[self rangeOfEntireDocument]]: 0;
}

loadDataRepresentation:

The original. Same story.

- (BOOL)loadDataRepresentation:(NSData *)data ofType:(NSString *)aType {
    BOOL success = NO;
    if ([aType isEqual: @"My Document Type"]) {
    if (documentTextView!=nil) {
        [self loadTextViewWithInitialData: data];
    } else {
        dataFromFile = [data retain];
    }
    success = YES;
    }
    return success;
}

Spaghetti. You gotta wonder who wrote this and if it was even thought out beforehand. There are a lot of ways to write this snippet correctly and the following is perhaps the best for several reasons.

-(BOOL)loadDataRepresentation:(NSData *)data ofType:(NSString *)aType {
    BOOL f = [aType isEqual:@"My Document Type"];

    if (f) {
        if (documentTextView)
            [self loadTextViewWithInitialData:data];
        else
            dataFromFile = [data retain];
    }
    return f;
}

Most importantly we keep but one exit. We also don't use braces where we're not supposed to. We make the code simpler so it's easier to read and maintain. And probably gain a few bytes object along the way. The obvious blooper in the original was in thinking you have two 'truth' values to worry about when you really only have one.

printDocument:

Another case of storing things you never need. The original:

- (void) printDocument:(id) sender {
    NSPrintOperation *printOperation = [NSPrintOperation printOperationWithView: documentTextView];
    [printOperation runOperationModalForWindow: documentWindow delegate: nil
        didRunSelector: NULL contextInfo: NULL];
}

The point is there's a stack variable assigned the return from printOperationWithView: except it's never used as such. This is totally different from the compiler temporary variable used to first catch the return. This is what the code should look like.

-(void)printDocument:(id)sender {
    [[NSPrintOperation printOperationWithView:documentTextView]
        runOperationModalForWindow:documentWindow delegate:0 didRunSelector:0 contextInfo:0];
}

rangeOfEntireDocument

Another classic. Before:

- (NSRange) rangeOfEntireDocument {
    NSInteger length = 0;
    if ([documentTextView string]!=nil) {
    length = [[documentTextView string] length];
    }
    return NSMakeRange(0,length);
}

Too many braces and too damn many variables. Like a worn out record. But here's another thing people just don't get: you don't call an object like that twice for the same result - that takes time and wastes money. Once you have the answer to the [documentTextView string] question you keep it - you don't ask again.

C is the fastest language in the world. By definition. But the slowest thing about C - or any language for that matter, about machine code itself - is the function call. The 'gosub'. This takes pushing a lot of stuff on the stack you'll need when control returns, putting other stuff there for the function you're calling, the other function popping it off the stack and pushing return stuff back on the stack, you popping your original stuff back off the stack - all for one function call.

So you obviously don't use function calls when you don't need them. And with Objective-C you raise the bar further: there are no function calls anymore - there are messages. They go through the Objective-C runtime. They're slower than C function calls - not a lot slower but still and all. So you don't want to waste clock cycles and code just because you're a boob.

And you don't use the inline NSMakeRange unless you have to - and here you definitely don't have to. An NSRange already has an NSInteger - an NSUInteger in fact so there's a possible overflow exploit in the author's code to make things even worse.

-(NSRange)rangeOfEntireDocument {
    NSString *s; NSRange range = {0, 0};

    if (s = [documentTextView string]) range.length = [s length]; return range;
}

searchUsingToolbarSearchField:

Before.

- (void) searchUsingToolbarSearchField:(id) sender {
    NSString *searchString = [(NSTextField *)[activeSearchItem view] stringValue];
    NSArray *rangesOfString = [self rangesOfStringInDocument:searchString];
    if ([rangesOfString count]) {
    if ([documentTextView respondsToSelector:@selector(setSelectedRanges:)]) {
        [documentTextView setSelectedRanges: rangesOfString];
    } else {
        [documentTextView setSelectedRange: [[rangesOfString objectAtIndex:0] rangeValue]];
    }
    }
}

Eek. After.

-(void)searchUsingToolbarSearchField:(id)sender {
    NSString *searchString = [(NSTextField *) [activeSearchItem view] stringValue];
    NSArray *rangesOfString = [self rangesOfStringInDocument:searchString];

    if ([rangesOfString count]) {
        if ([documentTextView respondsToSelector:@selector(setSelectedRanges:)])
            [documentTextView setSelectedRanges:rangesOfString];
        else
            [documentTextView setSelectedRange:[[rangesOfString objectAtIndex:0] rangeValue]];
    }
}

validateMenuItem:

Another case where preliminary thoughts become the final code. Except they shouldn't. Before:

- (BOOL) validateMenuItem: (NSMenuItem *) item {
    BOOL enabled = YES;

    if ([item action]==@selector(searchMenuFormRepresentationClicked:)
        || [item action]==@selector(searchUsingSearchPanel:)) {
    enabled = [self validateToolbarItem: activeSearchItem];
    }

    return enabled;
}

The key of course is you don't need no 'steekin' BOOL. If you ask for one the compiler will do its duty and give you one. But you're the fool, not the GCC. And you don't phone home more than once.

-(BOOL)validateMenuItem:(NSMenuItem *)item {
    SEL sel = [item action];

    return (sel == @selector(searchMenuFormRepresentationClicked:) ||
            sel == @selector(searchUsingSearchPanel:))?
        [self validateToolbarItem:activeSearchItem]: 1;
}

validateToolbarItem:

Same thing but worse. This is a real Italian starter. Original:

- (BOOL) validateToolbarItem: (NSToolbarItem *) toolbarItem {
    BOOL enable = NO;
    if ([[toolbarItem itemIdentifier] isEqual: SaveDocToolbarItemIdentifier]) {
    enable = [self isDocumentEdited];
    } else if ([[toolbarItem itemIdentifier] isEqual: NSToolbarPrintItemIdentifier]) {
    enable = YES;
    } else if ([[toolbarItem itemIdentifier] isEqual: SearchDocToolbarItemIdentifier]) {
    enable = [[[documentTextView textStorage] string] length]>0;
    }
    return enable;
}

But the same rules apply. After visiting Ricki Lake:

-(BOOL)validateToolbarItem:(NSToolbarItem *)toolbarItem {
    NSString *s = [toolbarItem itemIdentifier];

    return
        ([s isEqual:SaveDocToolbarItemIdentifier])? [self isDocumentEdited]:
        ([s isEqual:NSToolbarPrintItemIdentifier])? 1:
        ([s isEqual:SearchDocToolbarItemIdentifier])?
            ([[[documentTextView textStorage] string] length] > 0):
        0;
}

And So Forth

And so forth. The above exercise took 30 minutes to complete. It takes considerably longer to read and longer still to write. The savings are in bytes on disk and code maintainability. The above changes to the project will save approximately 600 bytes object code.

600 bytes out of 16000 doesn't seem a lot. About 4%. But results vary and this wasn't the worst of examples. Still: if you could better the efficiency of all the binaries on disk by 4% by devoting half an hour to each...

But that's the joke: if you did it right to start with you wouldn't need that extra half hour. And your development time would be faster than before too.

And in actual fact the above tweaked far more off the (i386) executable: it's 30040 bytes with the source files as provided by Apple and 27176 bytes once the code's been cleaned - a 10% improvement.

Further Reading

Get a dragon book. If you don't know what that is then ask around. It's got lots of pages. If you find it hard going it's OK. Just keep reading. Get out your copy of Kernighan Ritchie and read it again. Do all the exercises. Then read it again. And so forth.

About | ACP | Buy | Industry Watch | Learning Curve | News | Products | Search | Substack
Copyright © Rixstep. All rights reserved.