Re: [PATCH 04/12] check_everything_connected: use a struct with named options

From: Jeff King <peff@peff.net>
Date: Jul 16, 2016, 01:24 UTC
Message-ID: <20160716012431.GB30820@sigill.intra.peff.net>
In-Reply-To: <xmqqfuran50r.fsf@gitster.mtv.corp.google.com>

On Fri, Jul 15, 2016 at 11:13:40AM -0700, Junio C Hamano wrote:

Show 14 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > The number of variants of check_everything_connected has
> > grown over the years, so that the "real" function takes
> > several possibly-zero, possibly-NULL arguments. We hid the
> > complexity behind some wrapper functions, but this doesn't
> > scale well when we want to add new options.
> 
> I was kind of embarrassed to admit that I wasn't even aware that
> things got this bad, so I took a look at the history to realize that
> "grown over the years" above is a bit misleading statement.  It is
> not like many people over the years were doing something like this.
> There are only two commits that brought in this pattern with poor
> design taste.

Heh. It is easy to lose sight of such things when it is not your primary goal. You should see how horrible the code was that I wrote in my original iteration, before I went back and did this refactoring. I am embarrassed to have even written it in a rough draft. ;)

-Peff