Re: wordlist-refactor branch complete

On Fri, Sep 19, 2008 at 9:08 PM, Alex Rousskov
<rousskov_at_measurement-factory.com> 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