Re: pseudo-specs for a String class

Er.. That was a 'see you', as mangled by a BlackBerry.

On 9/2/08, Kinkie <gkinkie_at_gmail.com> wrote:
> Thanks for this extra feedback. I appreciate the advice everyone is giving.
> I'll have limited access to the net for the next couple of days so I
> won't be able to answer to the points you make right away, I should
> have the time to incorporate some of your suggestions by then.
> Are you!
>
>
>
> On 9/2/08, Alex Rousskov <rousskov_at_measurement-factory.com> wrote:
>> On Tue, 2008-09-02 at 05:10 +0200, Kinkie wrote:
>>> On Tue, Sep 2, 2008 at 12:21 AM, Alex Rousskov
>>> <rousskov_at_measurement-factory.com> wrote:
>>> > On Sun, 2008-08-31 at 14:07 +0200, Kinkie wrote:
>>> >
>>> >> I've gotten a bit forward, now I'm a bit at a loss about where to go
>>> >> next.
>>> >
>>> > I would move string, stream, and other formatting functions away from
>>> > this class.
>>> >
>>> > I would move searching and comparison functions away from this class.
>>> >
>>> > I would probably remove default conversion from char*. It buys you very
>>> > little in Squid low-level Buffer context, but can introduce serious
>>> > problems (we have seen some of that during the previous BetterString
>>> > attempts).
>>>
>>> I'm trying to make the class as useable as possible.
>>> Subclassing to add those operations is of course a possibility, I
>>> invite you to check the code out (it has a quite rich test/example
>>> section).
>>
>> Test/examples do not matter in this particular case. What matters is the
>> unexpected and expensive implicit conversions that are pretty much
>> guaranteed to appear in the real code if implicit conversion from char*
>> is allowed. We cannot avoid all implicit conversions, but I would still
>> recommend removing the implicit conversion from char* from any
>> buffering- or string-related class.
>>
>>> >> char *exportCopy(void);
>>> >> char *exportRefIKnowWhatImDoing(void);
>>> >
>>> > What do these return and how do they differ?
>>>
>>> The first exports a copy of the KBuf contents (potentially expensive
>>> but clear from the point of view of memory management), the latter
>>> exports a reference to the internal storage. Very cheap but quite
>>> dangerous as the underlying storage may be moved away. Thus the name
>>> expresses a contract by which the caller declaares to know what she's
>>> doing.
>>
>> Understood. Please document:
>>
>> - the mechanism for destroying/freeing a "safe" copy,
>> - whether the "safe" copy is null-terminated,
>> - the scope where it is safe to use the IKnowWhatImDoing reference.
>>
>>> >> KBuf& operator = (char const *S, u_int32_t Ssize);
>>> >
>>> > Does C++ have a tertiary assignment operator?
>>>
>>> of course not; please conosider that interface is a mock-up. Actual
>>> functions are:
>>>
>>> assign(KBuf &)
>>> operator = (KBuf &)
>>> assign (const char*, size_t)
>>> assign (const char*)
>>> operator = (const char *)
>>
>> OK. The conversion from char* string and lack of "const" comments apply
>> to all of these but the third one then.
>>
>>> >> const int operator [] (int pos);
>>> >
>>> > Useless without size() and probably does not belong to a low-level
>>> > buffer.
>>>
>>> size() has been implemented since.
>>> That operator returns -1 (EOF) on out-of-bounds. I agree it's probably
>>> useless, I'm know of throwing all possible ideas in, removing calls is
>>> always possible.
>>
>> I agree that removing something is always possible. On the other hand,
>> reviewing "all possible ideas" APIs is painful and not very productive.
>> Also, it is usually easier to add than to remove.
>>
>>> > There are a few places with methods arguments are references that are
>>> > missing "const". Please review.
>>>
>>> Ok. Are you referring to the mockup or did you check the actual code?
>>
>> I am referring to the "Current interface" blob you posted, asking about
>> the direction to go next. If the actual interface is not the "current
>> interface", please post whatever we should be looking at.
>>
>>
>> Four more comments on the "current interface" blob. Sorry if these are
>> irrelevant to the actual work.
>>
>> * Please document the results of invalid operations such as appending
>> more data than would fit or truncating more than there is. I would
>> recommend throwing an exception as the starting point, but whatever the
>> final/agreed solution is it needs to be documented and discussed.
>>
>> * bool isNull();
>>
>> Unlike for a pointer, being "null" is a strange state for a buffer.
>> Please consider renaming that method to isEmpty (if that is what you
>> meant). BTW, MemBuf::isNull is not isEmpty!
>>
>> * KBuf nextToken(const char *delim); //strtok()
>>
>> The above API would require removing the token from the object or
>> keeping the token pointer as additional data member. Neither approach
>> allows multiple concurrent string iterations. Is that intentional?
>> Should data iteration be external to the object holding the data?
>>
>> * Many methods that probably do not modify the object are missing
>> "const" at the end.
>>
>>
>>> As already discussed, I have no objections to splitting the class'
>>> interface in low-level and high-level.
>>> I'm currently taking the pragmatic approach that the high-level
>>> functions going to be almost always used anyways, so might as well
>>> clump them together.
>>
>> Not sure why clumping high- and low-level interfaces together is
>> pragmatic, but when do you expect to make the decision on whether the
>> final interface you recommend would be "clumped" or not? This decision
>> would affect a few key aspects of String/buffer classes, such as
>> construction and extraction, so it would be nice not to delay it for too
>> long.
>>
>> Thank you,
>>
>> Alex.
>>
>>
>>
>
>
> --
> /kinkie
>

-- 
    /kinkie