tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: style change: explicitly permit braces for single statements



>> 	if (ch == t.c_cc[VINTR]) {
>> 	} else {
>> 		if (ch == t.c_cc[VQUIT]) {
>> 		} else {
>> 			if (ch == t.c_cc[VKILL]) {
>> 			} else {
>> 			}
>> 		}
>> 	}

> For this, I would prefer

>  	if (ch == t.c_cc[VINTR]) {
>  	} else if (ch == t.c_cc[VQUIT]) {
>  	} else if (ch == t.c_cc[VKILL]) {
>  	} else {
>  	}

Sure.  So would I.  But that involves a subordinate clause (each if
except the first) that isn't brace-bracketed.  Note what I wrote before
the example: "[s]lavishly always adding [braces] makes [...]"; that was
an example of readability impairment resulting from insistence on
always brace-surrounding dependent statements under all circumstances.

> In this case, perhaps even a switch might better,

Except that can't be done as a switch, because the cases aren't
compile-time constant expressions.  The comment before it in the
original even calls it out as such:

                /*
                 * Scan for special characters.  This code
                 * is really just a big case statement with
                 * non-constant cases.  [...]
                 */

(sys/kern/tty.c, at least as of 1.227.4.2 - I simplified the code
significantly to not distract from my point).

>  	switch (ch) {
>  	case t.c_cc[VINTR]) {
>  		...do INTR processing...
>  		break;
>  	};
> [...]

That is definitely not C.  The syntax is wrong (it looks like a C
switch but with sh-style ) terminators on the cases) and the cases are
non-constant.  I chose that example specifically because it can't be
transformed into a (C) switch; it was the first example to come to mind
of a relatively unavoidable else-if chain in live code.  (My memory was
flaky; see below.)

That particular one can be transformed so it doesn't walk into the
right margin even if you _do_ insist on brace-bracketing, but it calls
for a different trick.  The first way that comes to mind is

	do {
		if (ch == t.c_cc[VINTR]) {
			...do INTR processing...
			break;
		}
		if (ch == t.c_cc[VQUIT]) {
			...do QUIT processing...
			break;
		}
		if (ch == t.c_cc[VKILL]) {
			...do KILL processing...
			break;
		}
		...etc...
	} while (0);

Is that better?  Sometimes, maybe.  tty.c uses gotos; in the simplified
form we've been discussing, that would be

	if (ch == t.c_cc[VINTR]) {
		...do INTR processing...
		goto endcase;
	}
	if (ch == t.c_cc[VQUIT]) {
		...do QUIT processing...
		goto endcase;
	}
	if (ch == t.c_cc[VKILL]) {
		...do KILL processing...
		goto endcase;
	}
	...
endcase:

but that's pretty close to the bottom of my own preference list.

Amusingly, the original of the example I cited isn't actually an
example of what I was trying to cite at all.  Perhaps I should have
made up something plausible but entirely fictitious, or gone and dug up
an example of a long else-if chain in real code somewhere.

/~\ The ASCII				  Mouse
\ / Ribbon Campaign
 X  Against HTML		mouse%rodents-montreal.org@localhost
 \ Email!	     7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B


Home | Main Index | Thread Index | Old Index