[Swan-dev] avoiding code churn

[Andrew Cagney]

On Thu, 29 Nov 2018 at 13:10, Andrew Cagney <andrew.cagney at gmail.com> wrote:
>
> The key word is 'immediate'.  For instance, the next commit cleaned up
> rehash_state() - dropped a no-longer-used parameter and tweaked the
> type of another.  I've a nice queue.  However each requires careful
> testing - I don't want to push a jumbo patch breaking something
> obscure.

I've just pushed the next change, it might provide a better
illustration of what I mean:

    IKEv2 NAT: replace convoluted way of coming up with responder's SPI

    Since the caller knows which SPI (zero or real) should be used, have
    the caller pass it in.

    Now-redundant parameters are dropped, simplifying calls.

While small, this isn't trivial (and my first attempt got one call wrong).

However, that's not what's interesting.  What is interesting is what I
decided to leave out - any changes to natd_hash() (for instance
passing ike_spis_t) - I felt that would have bloated the patch.

> On Wed, 28 Nov 2018 at 17:56, D. Hugh Redelmeier <hugh at mimosa.com> wrote:
> >
> > | commit 0f76a30e058b89c01417df6b937d928de0a446f6
> > | Author: Andrew Cagney <cagney at gnu.org>
> >
> > |     Avoid immediate code churn by #defining isa_[ir]cookie.
> >
> > Avoiding code churn has some disadvantages.  In general, I would avoid
> > avoiding it.
> >
> > 1) There are now two names for the same thing.  This is especially
> > dangerous when the thing is mutable: it's called aliasing.  It is a
> > serious source of confusion for programmers and compilers.  (I admit
> > that in this case, the compiler won't get confused because the
> > renaming is done by the preprocessor.)
> >
> > 2) readers need to know two ways to do something.  That's often worse
> > than one bad way.  It's certainly worse than one good way.
> >
> > 3) if the new form is better, why not get rid of all instances of the
> > old form, improving all the code.
> >
> > 4) if the new form has bugs, making the complete transition is more
> > likely to smoke them out.
> >
> > 5) a partial change adds to "technical debt".  We've already got
> > enough.
> >
> > Why would partially implementing a change be optimal?
> >
> > Perhaps if the new mechanism isn't yet complete so not all uses of the
> > old mechanism could be converted.
> >
> > Perhaps if a lot of reworking of each use of the old mechanism were
> > required it might be worth doing a bit at a time.
> >
> > If the problem is that it might interfere with other folks work that
> > hasn't been pushed, then I recommend not even starting the change.
> > _______________________________________________
> > Swan-dev mailing list
> > Swan-dev at lists.libreswan.org
> > https://lists.libreswan.org/mailman/listinfo/swan-dev