On Sun, Sep 21, 2008 at 1:36 PM, Amos Jeffries <[EMAIL PROTECTED]> 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 >>> <[EMAIL PROTECTED]> wrote: >>>> >>>> On Sat, 2008-09-20 at 13:31 +0200, Kinkie wrote: >>>>> >>>>> 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: >>>>>>> >>>>>>> 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
