> 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).
>
>> char *exportCopy(void);
>> char *exportRefIKnowWhatImDoing(void);
>
> What do these return and how do they differ?
I though they were
exportCopy == xstrdup() - or similar anyway.
other wone is just internal buffer export.
Kinkie, I'd make that const and re-work the users that would have it
otherwise.
>
>> KBuf& operator = (char const *S, u_int32_t Ssize);
>
> Does C++ have a tertiary assignment operator?
No. That should be a constructor.
KBuf out = KBuf(S,strlen(S));
>
>
>> 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().
>
>
>> const int operator [] (int pos);
>
> Useless without size() and probably does not belong to a low-level buffer.
>
Also, [] on a string should not return int. It should return whatever a
single character representation is, char or BYTE, etc.
There are two versions of this needed;
One: const char ...() const; for read-only access to a char. With tests
returning NULL char or other safe abortion for out-of-range requests.
The other: char ...(); for write-access, with full tests for string
expansion if necessary to store the new data.
>
> There are a few places with methods arguments are references that are
> missing "const". Please review.
>
Here are some hints...
>
>> 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();
() const;
>> KBuf& operator = (KBuf &S);
>> KBuf& operator = (char const *S, u_int32_t Ssize);
invalid. The code for this needs to use ... = KBuf(S, 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?
>> const int operator [] (int pos);
>> int cmp(KBuf &S); //strcmp()
booleans are all: ...()const;
>> 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
>> 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);
what about 'absent' case usually referred by signed -1.
>> }
These following are where all the tricky bits come in. What are they
actually needed for?
>> std::ostream & print (std::ostream &os); // for operator<<
>> void dump(ostream &os); //dump debugging info
>> static ostream & stats(ostream &os);
>> KBuf& appendf(const char *fmt, ...); //to be copied over from membuf
Obsolete if you want to implement a seperate KBufStream object taking
items and formatting them into a KBuf.
>>
>> on x86, sizeof(KBuf)=16
>>
>> Now I'm a bit at a loss as to how to best integrate with iostream.
Single inline-able function:
ostream &operator <<(ostream &os, KBuf const &S) {
os << KBuf.exportRefIKnowWhatImDoing();
}
The rest is handled by special handlers not related directly to KBuf as
Alex pointed out, but may use its other API calls to set data.
>> 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.
>
> HTH,
>
> Alex.
>
>
>
Received on Tue Sep 02 2008 - 03:37:25 MDT
This archive was generated by hypermail 2.2.0 : Tue Sep 02 2008 - 12:00:02 MDT