Re: [PATCH 2/4] basic priority queue implementation

From: Thiago Farina <tfransosi@gmail.com>
Date: May 20, 2011, 13:13 UTC
Message-ID: <BANLkTinojbxq=y3f-GPJsL7OH2OAJfW6zw@mail.gmail.com>
In-Reply-To: <20110520073821.GA3663@sigill.intra.peff.net>

On Fri, May 20, 2011 at 4:38 AM, Jeff King <peff@peff.net> wrote:

Show 12 quoted lines

> On Thu, May 19, 2011 at 09:47:38PM -0300, Thiago Farina wrote:
>
>> > +void queue_insert(struct queue *pq, void *item);
>> I'd rename this to queue_append as we add |item| to the end of the
>> array (like you did for sha1_array_append), opposed of inserting it at
>> some position/index.
>
> It's definitely not an append. The data structure is a priority queue,
> so the element is inserted within the heap at the proper position
> according to the comparison function (notice that we stick at the end,
> but then heapify_up).
>

OK, sorry. I didn't read the heapify_up part :(

Show 5 quoted lines

> Speaking of naming, though, the real problem is that this data structure
> should be called "pqueue" or something similar to indicate that it is
> not a simple FIFO. Unfortunately, the short-and-sweet "pqueue" is taken
> by openssl, which pollutes all over the global namespace.
>

Hum, yeah when I read the commit message I though about the name of the structure, but I didn't want to bother you with that. Probably you have considered the option of naming it 'priority_queue' too. Haven't you chose priority_queue, because you consider the name longer than necessary?

Show 6 quoted lines

> -Peff
>
> PS If you don't mind, please try to trim your quoted text a bit. Finding
> your 3-line paragraph amid 200 lines of quoted text was a little
> challenging. :)
>

OK. ;)