Re: ordered string-list considered harmful, was Re: [PATCH v3] Allow aliases that include other aliases

From: Jeff King <peff@peff.net>
Date: Sep 6, 2018, 20:49 UTC
Message-ID: <20180906204932.GA1482@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kbi=Ppkt4tYx+G_RDwp7e3+SU-LofZ-QzMsO2=zEPucsA@mail.gmail.com>

On Thu, Sep 06, 2018 at 01:04:18PM -0700, Stefan Beller wrote:

Show 17 quoted lines

> On Thu, Sep 6, 2018 at 12:12 PM Jeff King <peff@peff.net> wrote:
> >
> > On Thu, Sep 06, 2018 at 10:59:42AM -0400, Jeff King wrote:
> >
> > > > +           string_list_append(&cmd_list, *argv[0]);
> > >
> > > This will create an unsorted list. You'd have to use
> > > string_list_insert() here for a sorted list, or
> > > unsorted_string_list_has_string() in the earlier call.
> > >
> > > It's unfortunate that string_list makes this so easy to get wrong.
> >
> > This is getting really off-topic (since it sounds like we'd probably
> > want to use an ordered list here), but is it crazy to think that
> > basically every use of an ordered string list could just be a hashmap?
> 
> Does a hashmap guarantee an order?

No, it definitely doesn't.

I guess the reading-between-the-lines assumption that I didn't quite say is: I think most (if not all) of the users of sorted string lists don't actually care about a particular order. They just want efficient lookup.

Show 7 quoted lines

> I thought we had an example of an ordered list in the submodule code
> but could not find it, maybe it is gone already or did not rely on the order
> as I thought.
> 
> It turns out we make never use of a custom compare function in
> the stringlist, which helps gaining confidence this use case is nowhere
> to be found in the code.

Plenty of code uses the default strcmp. You can find users which assume sorting by their use of string_list_insert() versus _append(). Or ones that call string_list_sort(), of course.

-Peff