[Asterisk-Dev] [Rant] [long] - code style and quality

[Steven]

On Fri, 2005-05-06 at 15:54 -0700, Luigi Rizzo wrote:

> LIST OF COMMON STYLE BUGS
> 
> 1. in many cases we have blocks of this kind:
> 
> 	if (condition) {
> 		bla bla bla...
> 		... 100+ lines of code ...
> 	} else {
> 		ast_log(LOG_WARNING, "too bad, can't do that");
> 		return -1;
> 	}

I once submitted a patch to Mark to fox one specific instance of the
above. The trouble with those patches is that the diff doesn't make it
obvious that it is a simple change and with it being potentially several
pages long, it is one that isn't always accepted quickly or at all. 


> 3. Unused language features.
> 
>    The C language has the conditional expression operator, and there
>    is no reason not to use it when it can reduce the amount of
>    programming errors and ease readability.
> 
>    A sample from zt_read():
> 
> 	if (ps.rxisoffhook)
> 	{
> 		p->subs[index].f.subclass = AST_CONTROL_RADIO_KEY;
> 	}
> 	else
> 	{
> 		p->subs[index].f.subclass = AST_CONTROL_RADIO_UNKEY;
> 	}
> 
>    could be written as
> 
> 	p->subs[index].f.subclass = ps.rxisoffhook ?
> 		    AST_CONTROL_RADIO_KEY : AST_CONTROL_RADIO_UNKEY

If the example is just that simple, it is understandable. The trouble
comes when you need to do more than just one action. As soon as there is
more than one action, you are back to the original construct and it may
even be more difficult to get the patch in. 

Anything else I have cut is probably due to agreement.
-- 
Steven <critch at basesys.com>