Re: StringNG merge?

> Perl's split() is a nice function, but SBufSplit() delimiters-preserving
> behavior is ugly, especially if its purpose is to create simple
> tokenizers. Since SBufSplit is not used (unless I missed such uses), I
> recommend removing it. When we actually need something like this, you
> may add it back, of course, and we can then discuss whether it is the
> best tool for the job at that time.

Removed, as answered in previous mail.

>
>
>>>> +class StrList
>>>> +{
>>>> +public:
>>>> + StrList() { }
>>>
>>> This class is missing a description.
>>
>> It's an intermediate step, its purpose is to provide a path for
>> disambiguating String-used-as-a-list by the various strList*
>> functions.
>> It can be omitted from the merge.
>
> It is your call whether to include it or not. If it is not needed for
> the rest of the patch, consider removing it -- it is much better to
> evaluate a class when there are examples of how it is going to be used.
>
> If you decide to keep it, it must be properly documented and implemented
> (regardless of whether you plan to remove it eventually).

Documented as "work in progress, incomplete, don't use" and #if 0-ed.
Will be removed if you think this is not enough.

>>>> + StrList(SBufList s) {
>>>> + const SBuf sep(", ");
>>>> + SBuf b=SBufListJoin(s,sep);
>>>> + data_=b.toString();
>>>> + }
>>>> + StrList(const char *s) {
>>>> + data_=s;
>>>> + }
>>>> + StrList (SBuf s) {
>>>
>>> These single-parameter constructors are probably missing "explicit"
>>> keyword to prevent accidental (and expensive!) conversions. Please also
>>> check whether all of them need to copy their actual arguments instead of
>>> using references.
>>
>> Added.
>> They do. StrList an intermediate step, to find and collecr all the
>> client code which does implicit string-to-strlist conversions.
>> It's a side project, which can be skipped for now.
>
> That does not explain why "SbufList" and "SBuf" constructor arguments
> are passed by value. I am not saying they should not be. There may be a
> reason for those copies.

they can be made const& ; done so.

> If these classes are needed to detect implicit string-to-strlist
> conversions _and_ you plan to remove/replace such conversions (after
> they are detected), then perhaps these classes are not needed at all or
> do not need to be implemented (so that the linker will catch forgotten
> conversion cases)?
>
> If this is a separate project/step, consider removing it from StringNG
> submission so that there is fewer things to argue about.

commented out and marked as incomplete.

> Another concern about SBufList is that it provides nothing but two
> search functions, and those search functions do not rely on the storage
> being a specific std::list<>. There seems to be no reason to restrict
> that search functionality. Most likely, they should be implemented as
> global functions, accepting any container that we can iterate, AND/OR as
> a comparison function for a container type (depending on how they are
> actually used -- something, again, we cannot see right now).

yes, they could be made global. I am a bit less unsure about the
refactoring strategy.
The strategy I was aiming for IIRC was:
- define a specific type to sweep/mark all uses of String as a StrList
(while still keeping the underlying implementation a String)
- change the underlying implementation from a String to a SBufList
- profit

Suggestions are welcome on a more effective strategy (but it can wait
for another day; now I am aiming for SBuf, this is a side-project
which will become relevant when we start porting over String to SBuf).

--
    /kinkie