[asterisk-dev] code-cleanup concerns

Luigi Rizzo rizzo at icir.org
Fri Apr 14 16:59:42 MST 2006


On Fri, Apr 14, 2006 at 04:28:23PM -0700, Brian Degenhardt wrote:
> I'm a bit uncomfortable about some of these recent commits to trunk.
> While I'm all in favor for code cleanup, it should be taken in account 
> that changing things arbitrarily will break many patches sitting in 
> mantis.
> 
> Unless somebody can explain why for(;;) is inferior to do{}while(1), or 

i think for (;;)  or while () is a lot more readable than do{}while(1) because with the
former you know immediately that this is meant to be an 'infinite loop'
with the break condition to be looked up in the middle, whereas in the
do-while case you need to go to the very end (in some cases way way down)
to find out if there is any other exit condition.

> why removing curly braces on one-line if statements is worth the trouble 

on this there are different schools of thoughts, but my take in
general is that it makes the code a lot more compact and readable,
and as an important side effect, this makes repeated blocks of code
a lot more evident.

Both things are severe issues in the asterisk code, where replicated
code abunds and functions and blocks tend to be huge.

In fact, these removals are in preparation of some code restructuring
(including algorithmic simplifications) that have been in my branch
on svn for quite some time and that i am trying to merge in. Doing
the changes one piece at a time, rather than all at once, makes it
easier to separate functional from non-functional changes, and
identify bugs.

> of patch maintainers everywhere, I think this just does more harm than good.

unfortunately i have no solution to this.

We are all stalling on each other's patches to some degree, and as soon as
something gets put in, other patches may fail to apply. It has happened to
me as well as to others many many times for several large patches.
Hopefully as stuff gets merged in, and more people get commit access,
the delay for code to be merged will become lower and the problem
will disappear.

	cheers
	luigi

> cheers
> -bmd
> 
> svn-commits at lists.digium.com wrote:
> > Author: rizzo
> > Date: Fri Apr 14 17:48:05 2006
> > New Revision: 20169
> > 
> > URL: http://svn.digium.com/view/asterisk?rev=20169&view=rev
> > Log:
> _______________________________________________
> --Bandwidth and Colocation provided by Easynews.com --
> 
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>    http://lists.digium.com/mailman/listinfo/asterisk-dev



More information about the asterisk-dev mailing list