On Thursday, June 28, 2012 09:47:05 PM Alvaro Herrera wrote:
> Excerpts from Andres Freund's message of jue jun 28 14:20:59 -0400 2012:
> > Looks good now?
>
> The one thing I dislike about this code is the names you've chosen. I
> mean, ilist_s_stuff and ilist_d_stuff. I mean, why not just Slist_foo
> and Dlist_bar, say? As far as I can tell, you've chosen the "i" prefix
> because it's "integrated" or "inline", but this seems to me a rather
> irrelevant implementation detail that's of little use to the callers.
Well, its not irrelevant because you actually need to change the contained
structs to use it. I find that a pretty relevant distinction.
> Also, I don't find so great an idea to have everything in a single file.
> Is there anything wrong with separating singly and doubly linked lists
> each to its own file? Other than you not liking it, I mean. As a
> person who spends some time trying to untangle header dependencies, I
> would appreciate keeping stuff as lean as possible. However, since
> nobody else seems to have commented on this, maybe it's just me.
Robert had the same comment, its not just you...
It would mean duplicating the ugliness around the conditional inlining, the
comment explaining how to use the stuff (because its basically used the same
way for single and double linked lists), you would need to #define
ilist_container twice or have a third file....
Just seems to much overhead for ~100 lines (the single linked list
implementation).
What I wonder is how hard it would be to remove catcache.h's structs into the
implementation. Thats the reason why the old and new list implementation
currently is included all over the backend...
Greetings,
Andres
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
