[Debian-eeepc-devel] [patch] proposal: /etc/acpi/lib/notify.sh

[Cristian Ionescu-Idbohrn]

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