Excerpts from Andres Freund's message of jue jun 28 16:03:26 -0400 2012: > > 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.
Sure, at that point it is relevant. Once you're past that, there's no point. I mean, you would look up how it's used in the header comment of the implementation, and then just refer to the API. > > 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). Well, then don't duplicate a comment -- create a README.lists and refer to it in the comments. Not sure about the ilist_container stuff, but in principle I don't have a problem with having a file with a single #define that's included by two other headers. > 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... Yeah, catcache.h is a pretty ugly beast. I'm sure there are ways to behead it. -- Álvaro Herrera <alvhe...@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
