Re: [PATCH] pass constants as first argument to st_mult()

From: Jeff King <peff@peff.net>
Date: Aug 1, 2016, 21:11 UTC
Message-ID: <20160801211131.6ernsu74ohod2cin@sigill.intra.peff.net>
In-Reply-To: <xmqq8twgi4qp.fsf@gitster.mtv.corp.google.com>

On Mon, Aug 01, 2016 at 02:00:46PM -0700, Junio C Hamano wrote:

Show 11 quoted lines

> > Since multiplication is
> > commutative, it would be correct for st_mult() to just flip the order of
> > arguments it feeds to unsigned_mult_overflows().
> >
> > That may introduce the same inefficiency in other callsites, but I
> > wonder if it would be fewer.
> 
> "git grep -A3 st_mult \*.c" seems to tell me that the callsites with
> a constant in their first parameter are the majority (many are
> sizeof(something)).  The three places in the patch under discussion
> are the only places that got them in the different order.

Thanks for checking. I should have been less lazy and done it myself. If the majority are the other way, I agree that just fixing the minority is the best path forward.

Show 6 quoted lines

> *1* I have a slight suspicion that this is cultural, i.e. how
> arithmetic is taught in grade schools.  When an apple costs 30 yen
> and I have 5 of them, I was taught to multiply 30x5 to arrive at
> 150, not 5x30=150, and I am guessing that is because the former
> matches the natural order of these two numbers (cost, quantity) in
> the language I was taught.

You might be right. I was trying to figure out what is "natural" for me in these cases, but after thinking about it for 2 minutes, I'm pretty sure anything resembling "natural" is lost as I try to out-think myself. :)

-Peff