macOS [Cocoa] "Released when closed"

[MacRohde]

I have a question on "best style" regarding Cocoa architecture.

I have a controller that manages a small dialog window. When the controller is initialized the dialog is shown.

There's also a showDialog method. If the dialog is minimized or otherwise obscured the method throws the dialog to the front. If the nib hasn't been loaded it gets loaded. The code is like this:

-(void) showDialog {	
	if (!originalNumberTextField) {
		if (![NSBundle loadNibNamed:@"ConvertNumberToBase" 
							  owner:self])
		{
			NSLog(@"Error loading Nib for ConvertToBase dialog!");
			return;
		}
	}
	
	[[originalNumberTextField window] makeKeyAndOrderFront:self];
}

This works only if I in IB don't check the "Release when closed" check box under the window's attributes. If I do check it my outlet pointers (fx originalNumberTextField) are stale and I end up accessing bad memory, and then the program crashes.

I gather in some case it is fine not to release the window when closed, but in some cases it is a good idea to do (memory-wise).

So my questions are this:

1. If I choose to release the window when closing, how should I go about the situation since my pointers are stale? Do I need to listen for some notification for the release of the window and then set my outlets to nil?

2. Is there some Cocoa idiom I am missing on how to manage this?

 

[kainjow]

Without knowing further how your class is setup, I'd suggest adding an IBOutlet to the window directly, otherwise it could be getting released too early since there is no reference to it keeping it around. Or just make your class a subclass of NSWindowController which manages all this (nib loading, showing the window, etc) for you.

Edit: my theory is probably incorrect since I can't replicate the issue, so I'd suggest posting some more code.

 

[MacRohde]

Hi kainjow.

As mentioned my code works if I make sure the window is not released when it is closed. I wanted tho' to get it to work without that setting, so I refactored my code like this (and it works as well) - I would like some criticism, especially if I am not doing it the Cocoa way.

In my MainMenu.xib I have a controller (type ApplicationController) that "listens" for the proper menu click and instantiates the dialog window controller (type ConvertNumberToBaseController). This controller is also the File's Owner of the nib for the dialog window. ApplicationController is declared and defined like this:

#import <Cocoa/Cocoa.h>
#import "ConvertNumberToBaseController.h"


@interface ApplicationController : NSObject {
	ConvertNumberToBaseController * convertNumberToBaseController;
}

-(IBAction) showConvertNumberToBaseDialog:(id)sender;

@end

#import "ApplicationController.h"
#import "ConvertNumberToBaseController.h"


@implementation ApplicationController

-(void) dealloc {
	if (convertNumberToBaseController != nil) {
		[convertNumberToBaseController release];
	}
	
	[super dealloc];
}

-(IBAction) showConvertNumberToBaseDialog:(id)sender {		
	if (convertNumberToBaseController == nil) {
		convertNumberToBaseController = [[ConvertNumberToBaseController alloc] init];	
	}
	else {
		[convertNumberToBaseController showDialog];
	}
}

@end

When the proper menu item is clicked showConvertNumberToBaseDialog is called.

The ConvertNumberToBaseController is defined like this

@interface ConvertNumberToBaseController : NSObject {
	IBOutlet NSTextField * originalNumberTextField;
	IBOutlet NSPopUpButton * fromBasePopUpButton;
	IBOutlet NSPopUpButton * toBasePopUpButton;	
	IBOutlet NSTextField * resultLabel;	
}

-(IBAction) convertNumber:(id)sender;
-(void) showDialog;

@end

#import "ConvertNumberToBaseController.h"
#import "NumbersUtil.h"


@implementation ConvertNumberToBaseController

#pragma mark -
#pragma mark Initializtion and setup

-(void) loadUi {
	if (![NSBundle loadNibNamed:@"ConvertNumberToBase" 
						  owner:self])
	{
		NSLog(@"Error loading Nib for ConvertToBase dialog!");
		return;
	}
}

-(id) init {
	if (self = [super init]) {
		[self loadUi];		
	}
	return self;
}

#pragma mark -
#pragma mark Action handlers

-(IBAction) convertNumber:(id)sender {
	NSString * originalNumber = [originalNumberTextField stringValue];
	int fromBase = [fromBasePopUpButton indexOfSelectedItem] + 1;	
	int toBase = [toBasePopUpButton indexOfSelectedItem] + 1;
	
	if (originalNumber == 0 || fromBase == 0 || toBase == 0) {
		return;
	}
	
	NSString * convertedNumber = [NumbersUtil convertNumber:originalNumber InBase:fromBase ToBase:toBase];
	[resultLabel setStringValue:convertedNumber];
}

#pragma mark -
#pragma mark UI methods

-(void) showDialog {	
	if (!originalNumberTextField) {
		[self loadUi];
	}
	
	[[originalNumberTextField window] makeKeyAndOrderFront:self];
}

#pragma mark -
#pragma mark Window delegate methods

-(void) windowWillClose:(NSNotification *)notification {
	originalNumberTextField = nil;
	fromBasePopUpButton = nil;
	toBasePopUpButton = nil;	
	resultLabel = nil;
}


@end

So, when the window is closed it is released, and so is all the objects in the nib. I listen for the close event, and set my outlets to nil. This way when I check for nil in showDialog it works as expected.

Would this be a correct and proper way to do it?

 

[kainjow]

Ha I don't know what I was thinking with my original post...

Anyways, yes, the outlets are going corrupt because their original values are no longer around, but they haven't been set to nil, so they're just pointing to some random object/value in memory.

Unless you 100% know for a fact that releasing the window on close is going to make a noticeable difference in your program, I would leave that option off. For now it's just an unnecessary optimization.

But if you really wanted to do it, I think the best way would be to do what you mentioned - set the objects to nil when the window is closing via the window's windowWillClose: delegate method.