Re: Strategy

On Sun, Sep 21, 2008 at 1:36 PM, Amos Jeffries <squid3_at_treenet.co.nz> wrote:
> Alex Rousskov wrote:
>>
>> On Sun, 2008-09-21 at 01:05 +0200, Kinkie wrote:
>>>
>>> On Sat, Sep 20, 2008 at 9:26 PM, Alex Rousskov
>>> <rousskov_at_measurement-factory.com> wrote:
>>>>
>>>> On Sat, 2008-09-20 at 13:31 +0200, Kinkie wrote:
>>>>>
>>>>> 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:
>>>>>>>
>>>>>>> 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.
>>>>
>>>> Understood. So we have a few options here:
>>>>
>>>> 0) Do nothing now. Continue to use C-ish WordList until better String is
>>>> available. Decide what to do next at that time.
>>>>
>>>> 1) Polish and implement new C++ WorldList class that still uses char*
>>>> words. Convert all user code to use that new class. Later, throw away
>>>> that class, replace with some String-based list.
>>>>
>>>> 2) Use existing std::list<SquidString> or similar (i.e., use a standard
>>>> list and the existing SquidString class). Convert all user code to use
>>>> std::list<SquidString>. When proper String is available, replace
>>>> SquidString with String and, possibly, std::list with StringList.
>>>>
>>>> Out of these options, (1) seems to make least sense. Why spend hours on
>>>> inventing a new class that is going to be replaced with something
>>>> better, especially if that something better already exists?
>>
>>> The strategy behind the approach I've taken is: first fix the callers
>>> by making a reasonably OO callee, upsetting the callees as little as
>>> possible. Then make the callee better.
>>
>> IMO, spending a lot of time on slightly improving large volumes of
>> working code that should be and will be rewritten is a waste. I
>> understand that it is mostly your time that is being spent here, but
>> since we all are supposed to review the code and fix bugs, we are all
>> affected.
>>
>>> We have plenty of places where the C heritage shows up, and aiming for
>>> the perfect solution is a more difficult task than doing fixing things
>>> in steps. In this case, I did not invent a new class, but simply ported
>>> the
>>> existing interface to C++.
>>
>> Right. What I am asking is why change one bad interface to another bad
>> interface when a good interface is already available? Will it be a
>> little more difficult to put that good interface in? Yes. Does that
>> imply we should spend time migrating from one bad solution to another
>> poor solution? No. Will the effort to port old code to the good
>> interface pay off? Probably, because the good interface will stay for a
>> long time, will accommodate new String, and will help to fix bugs and
>> write better code.
>>
>>> I'd like to understand a bit better what's the forward strategy the
>>> project wants to take tho, as it seems that my development strategy
>>> does not seem to fit well with the overall project's.
>>
>> In general, when big/fundamental code pieces are modified, there should
>> be consensus how to modify them before the development work starts. If
>> there is no consensus or no quorum for consensus, the developer can
>> proceed at their own risk.
>>
>>> What I'm doing is, take a wart and improve it a bit. Maybe not in the
>>> best possible way, but hopefully improve it some. Yet I always seem to
>>> be missing something. In some cases I agree it's something which has
>>> to be fixed before merging can be considered (see the refcounting in
>>> SBuf), sometimes it's just a not-perfect piece of code, but which
>>> still is an improvement over the wart.
>>
>> I think your problems with String are different from the current
>> problems with WordList, but there is some overlap (other than me bugging
>> you). If I may, I can suggest the following general approach:
>>
>> * Look for simpler warts with localized impact. We have plenty of them
>> and your energy would be well spent there. If you have a choice, do not
>> try to improve something as fundamental and as critical as String.
>> Localized single-use code should receive a lot less scrutiny than
>> fundamental classes.
>>
>
> Agreed, but that said. If you kinkie, picking oe of the hard ones causes a
> thorough discussion, as String has, and comes up with a good API. That not
> just a step in the rght direction but a giant leap. And worth doing if you
> can spare the time (months in some cases).
> The follow on effects will be better and easier code in other areas
> depending on it.

That's exactly my purpose.

>> * When assessing the impact of your changes, do not just compare the old
>> code with the one submitted for review. Consider how your classes stand
>> on their own and how they _will_ be used. Providing a poor but
>> easier-to-abuse interface is often a bad idea even if that interface is,
>> in some aspects, better than the old hard-to-use one.
>>
>>> Noone else is tackling the issues that I'm working on. Should they be
>>> left alone? Or should I aim for the "perfect" solution each time?
>
> Perfect varies, and will change. As the baseline 'worst' code in Squid
> improves. The perfect API this year may need changing later. Aim for the
> best you can find to do, and see if its good enough for inclusion.
>
> for example, Alex had no issues with wordlist when it first came out. Its
> only now that String is improved so much that alternatives are appearing for
> wordlist.

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.

> This is one major reason I aim for the core classes first, despite their
> difficulty and wider initial effects, and whether or not they end up being
> changed now. If the ideas of _their_ improvement can provide baseline guides
> for other how other objects might soon be used t affects those other
> objects. (I've left URL and header parsers alone waiting on string for a
> year now).

>> I hope this email answers these questions, but I am sure other opinions
>> will vary.
>>
>>> But what is "perfect"?
>>
>> There is no definition of "perfect", of course, but the acceptance bar
>> rises with the number of current and expected users of the code...
>>
>>> I'm learning as I go - that's one of the reasons
>>> why I make very public RFCs..
>>
>> We all learn. I am excited to see you making progress, and I hate to
>> inflict pain, but my options are rather limited once you submit
>> something for a review.
>>
>> To overextend this learning metaphor, a good teacher would not have
>> given you the assignments you have chosen yourself! I am in no position
>> to teach, but I did provide a few project ideas below, FWIW.
>>
>>> So please, tell me what you consider the best way I can help - I'm
>>> obviously not doing well enough.
>>
>> You are actually helping plenty, just not with those two unfortunate
>> projects!
>
> For me String is a big help. Maybe not a good choice for beginner, but a big
> help. It opens several other projects for beginning.

Thanks, I appreciate it.

>> Here are a few personalized ideas, in no particular order:
>>
>> * Port Squid2 features missing in Squid3 (some are mentioned below).
>> * http://wiki.squid-cache.org/Features/HTTP11 (I can provide access to
>> Co-Asvisor to update the list of current violations, some mentioned
>> below)
>
> I'd put the next co-advisor test at just post-branch/PRE1.
>
>> * http://wiki.squid-cache.org/Features/CacheDirFailover
>> * http://wiki.squid-cache.org/Features/StaleIfError
>> * http://wiki.squid-cache.org/Features/StaleWhileRevalidate
>> * http://wiki.squid-cache.org/Features/Gzip (in two weeks)
>> * http://www.squid-cache.org/bugs/show_bug.cgi?id=2424
>> * http://www.squid-cache.org/bugs/show_bug.cgi?id=2351
>> * http://www.squid-cache.org/bugs/show_bug.cgi?id=2344
>> * http://www.squid-cache.org/bugs/show_bug.cgi?id=1200
>> * http://www.squid-cache.org/bugs/show_bug.cgi?id=2038
>> * http://www.squid-cache.org/bugs/show_bug.cgi?id=2055
>> * http://www.squid-cache.org/bugs/show_bug.cgi?id=2042
>> * http://www.squid-cache.org/bugs/show_bug.cgi?id=2314
>> * http://www.squid-cache.org/bugs/show_bug.cgi?id=2322
>> * http://www.squid-cache.org/bugs/show_bug.cgi?id=2463
>>
>> For some of the above, I have provided patches that need review,
>> polishing, and/or testing. I am adding those to bugzilla but not all
>> reports have been updated.
>>
>> I suspect others may have different personalized lists of projects and
>> even consider the String project appropriate...
>
> Yes.

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?

Continuing work on String/SBuf is a possible target I am willing to
commit to, but if someone wants to take over, I won't object.

Thanks.

-- 
 /kinkie