Bad Style Guides

This is the AntiPattern for a "SaneSubset". Unless we're talking about Java [tongue-in-cheek], most languages' features have a reasoned, peer-reviewed purpose. In fact, you can create gobs of unreadable, buggy code by adhering to a strict standard. Some C++ shops abhor templates, but that leads to duplicating common structures like collections and MixIns. Often, the duplications are CopyAndPasteProgramming and are riddled with bugs. So, exact definitions are evil. Guidelines are more useful. Idioms and patterns are even better. -- SunirShah

Readability trumps all style rules.

Agreed. ListenToTheCode implies CodeSings. AnalogiesFromMusic have always struck me as the most effective inspiration in this area. Note that there are many "rules" that you should not break as you compose or improvise. You can't change key every beat of every bar. But just as you can't make great music merely by following rules sometimes you have to break a rule to create the most beautiful or appropriate code. See also JazzProgrammer -- RichardDrake


Here are examples of C++ BadStyleGuides: Always test a pointer for NULL before dereferencing it. That's useless because you only need to test the first appearance of a suspicious pointer.

Griping about idiotic "style guides" is healthy, but none of your examples are insane enough. For some their educations have wired a "SaneSubset" into their reflexes. Not so for others...


Always test a pointer for NULL...

Consider these alternatives:

Always test an index...

Same idea: encapsulate the array in a template; let it do the check for you.

Don't define the body of a friend function inside a class...

If the code is short, it lives just as well inside the class definition as outside of it. Put it wherever it's the most readable.


First, allow me to show where these rules can get silly (Antithesis) before we get around to Synthesis. The suggestion to wrap low-level constructs in templates is a good idea. Stick with it. When you can't, or abhor templates (grrrrr)...

Always test a pointer for NULL before dereferencing it.

Anti-example 1 (doubletake)

 assert( pFoo );
 pFoo->Bar();

assert( pFoo ); // again. *yawn* pFoo->Baz();

Anti-example 2 (checking against NULL returns from new is similar to malloc()

 Foo pFoo = new Foo;
 assert( pFoo ); // new NEVER returns NULL unless your compiler sucks.

// It's "ok" to use pFoo because we've tested against NULL! pFoo->Bar();

Anti-example 3 (checking for NULL before delete--don't! delete NULL; is ok)

 assert( pFoo );
 delete pFoo;

Always test an index against the extent of an array before subscripting it.

Anti-example 1 (ASCIIZ strings don't come with extents.)

 // Not the coolest strupr(), but good enough for this example
 char *strupr( char *sz )
 {
	assert( sz );
	for( int i = 0; sz[i]; i++ )
	 sz[i] = toupper( sz[i] );
	return sz;
 }

Anti-example 2 (What extent? We don't need to know; let the caller deal with it.)

 void doSomethingFunkyToElement( Foo anArray[], int iIndex )
 {
	...
	... anArray[iIndex] ...
	...
 }

Don't define the body of a friend function inside a class that befriends it.

Anti-example (forcing template friends to instantiate for only the templates)

 template<class T>
 class Foo
 {
	...
	friend bool Bar( Foo<T> const &a, Foo<T> const &b )
	{
	 ...;
	}
	...
 };

template<> class Foo<Baz> { ... // No Bar() defined. Baz Foos can't be Bar()d };

Otherwise, you could Bar a Baz Foo, which isn't what you want.

Round up

Some of those examples may have seemed contrived. Well, they were, but they illustrate that you're preventing people from doing what they need to do in some cases. And you can't be sure those techniques aren't the simplest solutions in all cases. Indeed, the test for NULL from new or before delete is really annoying.

In the case of verifying the index, this will lead people pass parameters into methods/functions that will only be used in assert()s. Not good. Don't do that. This is what I meant by using guidelines, but nothing strict.

Also, I don't think this is really what "SaneSubset" means. SaneSubset means using a subset of the language features, not the generative possibilities. You have to do that anyway because there are an infinite number of expressions possible in most languages.

Synthesis

If you want to make guidelines for bug-proof code, make a parent page to SelfDocumentingCode called maybe BugProofCode? or DefensiveCodingStandards? or even generically CodingStandards. Some of the SelfDocumentingCode pages are really bug-proofing and should be moved.

-- SunirShah
The examples make me think of a way of writing code that I've often used but haven't got a good name for. Its when I write code that I know doesn't always work and then start adding all the exceptional cases that make sure it works afterwards. For example write code C that breaks when a point p is NULL. Then think about what it means for p to be null. Change code to something avoids the problem:
	if(p) C
	else do_something_useful_instead_of_crashing
or
	while( p )
		C
	//p is null so....

Or another version is to write code for handling items in an array in a patently wrong way
	for(;;)
		C
and then fill in the blanks in the for to make sure the C works.

A final variation (published years ago) was ErrorsBecomeFeatures.

It is of course more sophisticated than this. Is this a known pattern?

-- DickBotting

EditText of this page (last edited September 22, 2003) or FindPage with title or text search