Re: pseudo-specs for a String class

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).

>> 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.

>> 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 *)

>> KBuf consume(u_int32_t howmuch); //from MemBuf
>
> Since you are not just consuming but creating a new buffer, I would call
> this split(), move(), or extract().

I imported the name from MemBuf. I have no objections about changing
the function name, though.
What's everyone's take?

>> 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.

> 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?

>> Current interface:
>>
>> class KBuf {
>> KBuf();
>> KBuf(const KBuf &S);
>> KBuf(const char *S, u_int32_t Ssize);
>> KBuf(const char *S); //null-terminated
>> ~KBuf();
>> bool isNull();
>> KBuf& operator = (KBuf &S);
>> KBuf& operator = (char const *S, u_int32_t Ssize);
>> KBuf& operator = (char const *S); //null-terminated
>> KBuf& append(KBuf &S);
>> KBuf& append(const char * S, u_int32_t Slen);
>> KBuf& append(const char * S); //null-terminated
>> KBuf& append(const char c); //To be removed?
>> KBuf& appendf(const char *fmt, ...); //to be copied over from membuf
>> std::ostream & print (std::ostream &os); // for operator<<
>> void dump(ostream &os); //dump debugging info
>> const int operator [] (int pos);
>> int cmp(KBuf &S); //strcmp()
>> bool operator == (const KBuf & S);
>> bool operator <(KBuf &S);
>> bool operator >(KBuf &S);
>> void truncate(u_int32_t to_size);
>> KBuf consume(u_int32_t howmuch); //from MemBuf
>> void terminate(void); //null-terminate
>> static ostream & stats(ostream &os);
>> char *exportCopy(void);
>> char *exportRefIKnowWhatImDoing(void);
>> KBuf nextToken(const char *delim); //strtok()
>> KBuf substr(u_int32_t from, u_int32_t to);
>> u_int32_t index(char c);
>> u_int32_t rindex(char c);
>> }
>>
>> on x86, sizeof(KBuf)=16
>>
>> Now I'm a bit at a loss as to how to best integrate with iostream.
>> There's basically three possibilities:
>>
>> 1. KBuf kb; kb.append(stringstream &)
>> cheapest implementation, but each of those requires two to three
>> copies of the contents
>
> I do not think your buffer should know about stringstream.
>
>> 2. Kbuf kb; stringstream ss=kb->stream(); ss<< (blah).
>> this seems to be the "official" way of extending iostreams;
>> performed by making KBuf a subclass of stringbuf.
>> extends sizeof(KBuf) by 32 bytes, and many of its calls need to
>> housekeep two states.
>
> Do not do that, at least not for now. Your buffer is not a stream and
> probably is not a stream buffer either.
>
>> 3. Kbuf kb; stringstream ss=kb->stream(); ss << (blah)
>> performed by using an adapter class. The coding effort starts to
>> be quite noticeable, as keeping the stringbuf and the KBuf in sync is
>> not trivial.
>
> Same as #2.
>
>> 4 Kbuf kb<<(blah). requires kbuf to be a subclass of an ostream.
>> there's a significant coding effort, AND baloons the size of KBuf
>> to 156 bytes.
>
> #4 does not really require your class to be a child of ostream, but I do
> not think we need this either.
>
>
>> What's your take on how to better address this?
>
> IMO, you should not "address" formatted output in your buffer.
>
> If you insist on placing low-level buffering functions and high-level
> string functions together, then you can add a few << operators that
> would append a few commonly used types to the buffer. Adding those
> operators will not increase the memory footprint of your class.

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.

Thanks for your feedback.

-- 
 /kinkie