On Fri, 16 Apr 2010, Bob Wilkinson wrote: > On Wed, Apr 14, 2010 at 12:39:21AM +0200, Cristian Ionescu-Idbohrn wrote: > > > > > And some comments about the changes. > > > > > > This change: > > > > > > - CATEGORY=$1 > > > - MSG=$2 > > > > You might have misses these comments: > > > > + # $1 - category > > + # $2 - message > > + # $3 - ??? > > + # $4.. - ??? > > > > specially this one: > > > > + # positional parameters never change during the course of this function > > > > > and $MSG -> $2 and $CATEGORY -> $1 > > > > > > is making the whole patch bigger (and mixed) and I don't see the > > > benefits, apart from using $1, $2,.... > > > > Marginally bigger, yes. But those variables are not needed. And then, I > > documented the arguments, didn't I? > > I am not as concerned about the size of code as I am about > legibility and ease of comprehension.
Yes. I generally agree. But, in this particular case, creating new variables and moving values around comes at a price. I would ruther waste performance on error handling, as troubleshooting complex scripts can be painful. > I think that to name the positional parameters at the start of the > function produce code which is much more readable. Right. But, again, there's a performance price you'll have to pay. The shells are slow. Performance in eeepc-acpi-scripts context is important. > The increase in legibility is, I believe, directly proportional to > both the number of parameters and the length of the code. Sure, but not always. Several other things are important too, and finding the proper balance is not easy. > While I may read the comments at the top of the code, I then have > to memorise the mapping while reading the code. This is potentially > error-prone. If I can read the named parameters throughout the code > then I no longer have this potential for miscomprehension. Yes. But see above. > Code is read many more times than it is written. I couldn't agree more. Cheers, -- Cristian _______________________________________________ Debian-eeepc-devel mailing list [email protected] http://lists.alioth.debian.org/mailman/listinfo/debian-eeepc-devel
