On Fri, Sep 19, 2008 at 9:08 PM, Alex Rousskov <[EMAIL PROTECTED]> wrote: > On Fri, 2008-08-22 at 10:24 +0200, Kinkie wrote: > >> The wordlist-refactor branch is from my point of view feature-complete. >> It can be found in Launchpad, at >> https://code.launchpad.net/~kinkie/squid/wordlist-refactor. >> I'm not posting a bundle to propose merging into trunk at this time, >> because despite the patch being quite self-contained, some core >> developers have expressed concerns about it potentially introducing >> subtle bugs, and suggested it to be merged after branching 3.1. >> >> Here's the branch summary: >> Purpose of this branch is to migrate wordlist from a C-style interface >> with some c++ coating into a proper c++ object. It does not aim to >> increase the performance of the implementation, but to have cleaner >> semantics for the callers. >> This branch is an interim step on the way to a proper >> template-library-based class. Collateral advantages include unit >> tests, doxygen documentation and naming convention compliance. >> >> >> I appreciate anyone taking a peek at it and posting any comments. > > *B1* Should we just use std::list or another standard container instead? > It seems like a standard container provides everything the new WorldList > wants to provide. Is this a performance optimization?
No. This implementation is an intermediate step, migrating to a standard container is the eventual aim. The whole concept should probably also be phased out in favour of a SBufList. > *B1* The class declaration needs a brief description of the class: What > is a Word? What kind of List (singly or doubly linked, for example)? > Does the List own Words? Ok. It's a refactoring of the behaviour of C wordlist. > *B1* WorldList is not an algorithm so it should not be in > src/algorithms/ That's me misunderstanding the conventions again :( Will re-move to src/. > There are more lower-level problems, but I think we should resolve the > first two before proceeding further. -- /kinkie
