[Adium-devl] Early Returns and the Coding Style Docs

[David Smith]

	Hi everyone,

	I was looking at a block of code, and noticed that using early  
returns could make it, imo, much more readable. Basically, less  
indentation depth, less masses of closing braces to try to match up,  
and less scoping oddities (like having to declare replacementMessage  
outside the if). However, our coding guidelines say to avoid early  
returns. I can see the reasoning behind it (single exit point  
functions do have some appeal), but I think we make too many sacrifices.
	I do agree with the guidelines in that stuff like while() { while()  
{ if() { return foo; } } } return bar; should be avoided though. I  
see early returns as mostly being useful for bypassing method bodies  
when preconditions aren't met, such as below.
	What do people think of this? Am I in a minority here in liking  
early returns? (I know I'm a minority in liking braces on their own  
lines, I'm not even trying to argue that one anymore ;) )

Before:

- (NSAttributedString *)filterAttributedString:(NSAttributedString *) 
inAttributedString context:(id)context
{
	NSMutableAttributedString	*replacementMessage = nil;

	if (inAttributedString) {
		NSRange					linkRange = NSMakeRange(0,0);
		unsigned					index = 0;
		unsigned					stringLength = [inAttributedString length];

		replacementMessage = [inAttributedString mutableCopy];

		while (index < stringLength) {
			if (![replacementMessage attribute:NSLinkAttributeName  
atIndex:index effectiveRange:&linkRange]) {
				/* If there's no link at this index already, process it via the  
hyperlinkScanner to see if there should be one.
				 * We don't process existing links because (a) it would be  
duplicative effort and (b) we might mess up a link which had
				 * a linkable item within its text, such as "Check out the new  
story at adiumx.com" linked to an adiumx.com page.
				 */
				NSAttributedString	*replacementPart = [hyperlinkScanner  
linkifyString:[inAttributedString  
attributedSubstringFromRange:linkRange]];
				[replacementMessage replaceCharactersInRange:linkRange
										withAttributedString:replacementPart];
				stringLength -= linkRange.length;
				linkRange.length = [replacementPart length];
				stringLength += linkRange.length;
			}
			
			// increase the index
			index += linkRange.length;
		}
	}
	
	return (replacementMessage);
}


After (as well as a bit of other cleanup):

- (NSAttributedString *)filterAttributedString:(NSAttributedString *) 
inAttributedString context:(id)context
{
	if(!inAttributedString) return nil;
	
	NSMutableAttributedString	*replacementMessage = [inAttributedString  
mutableCopy];
	NSRange					linkRange = NSMakeRange(0,0);
	unsigned					stringLength = [replacementMessage length];

	for (int i = 0; i < stringLength; i += linkRange.length) {

		if ([replacementMessage attribute:NSLinkAttributeName atIndex:index  
effectiveRange:&linkRange])
			continue;
		
		/* If there's no link at this index already, process it via the  
hyperlinkScanner to see if there should be one.
		 * We don't process existing links because (a) it would be  
duplicative effort and (b) we might mess up a link which had
		 * a linkable item within its text, such as "Check out the new  
story at adiumx.com" linked to an adiumx.com page.
		 */
		NSAttributedString	*replacementPart = [hyperlinkScanner  
linkifyString:[inAttributedString  
attributedSubstringFromRange:linkRange]];
		[replacementMessage replaceCharactersInRange:linkRange
								withAttributedString:replacementPart];
		stringLength -= linkRange.length;
		linkRange.length = [replacementPart length];
		stringLength += linkRange.length;
	}
	
    	return replacementMessage;
}

	There's also a side issue in that *somehow* 	replacementMessage =  
[inAttributedString mutableCopy]; is failing, claiming that it's  
calling -initWithAttributedString: with a nil argument. Dunno how  
that happens.

																David Smith