On Sun, 2008-09-21 at 22:26 +0200, Kinkie wrote:
> Still, WordList is ready for merging, while String a long way off.
> It's build-tested, it's run-tested, and it's a fairly straightforward
> porting of the current codebase.
> It may be a waste of time in the end, but if it's mergeable, not
> merging it makes it doubly so.
WordList class, in its current shape, is not something I would like to
see used in Squid, primarily because it violates several basic
requirements for a good C++ class. It even violates the only two
mandatory requirements listed on the Squid3CodingGuidelines wiki page!
WordList widespread use in the code is an aggravating factor in this
case.
If others insist that this work must continue now, I may resume the
review to eliminate the bugs I see. I would very much prefer not to be
forced to spend time on that because I doubt we need that custom class
at all.
> Okay, so what next?
>
> Should I fix the major issues Alex has mentioned in WordList
> (documentation and moving out of algorithms/) and merge it, or abandon
> it?
I do not know what you mean by merging, but my recommendation is to
suspend your WordList project until we have a better class for a word
than "char*". At that time, I would return to the question whether we
can just use std::list or similar.
HTH,
Alex.
Received on Sun Sep 21 2008 - 22:02:29 MDT
This archive was generated by hypermail 2.2.0 : Mon Sep 22 2008 - 12:00:04 MDT