incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anton Pevtsov" <Ant...@moscow.vdiweb.com>
Subject RE: Tests for string non-members - operators
Date Tue, 30 May 2006 15:10:18 GMT
Martin Sebor wrote:
> We should exercise long strings of more varying length rather than
just 2048 characters (give or take a few). 
[...]
> initial or final length is close to the following values: 128, 207,
334, 540, 873, 1412, 2284, and 3695.

I updated several tests and added the test cases containing the strings
with special length.
The diffs are here:
http://people.apache.org/~antonp/stdcxx05302006/

Thanks,
Anton Pevtsov


-----Original Message-----
From: Martin Sebor [mailto:sebor@roguewave.com] 
Sent: Friday, May 26, 2006 22:30
To: stdcxx-dev@incubator.apache.org
Subject: Re: Tests for string non-members - operators


Anton Pevtsov wrote:
> Martin Sebor wrote:
> 
>>Would you mind terribly merging all the [in]equality and relational
> 
> tests and keep just the one for operator+() separate?
> 
> The merged operators test (21.string.operators) and separate test for
> operator+ are here:
> http://people.apache.org/~antonp/stdcxx05262006/

Great! Thanks!

A few comments/suggestions/requests:

The name of the test in the comment at the top of the .cpp file should
match the file name, i.e., 25.string.op.plus.cpp. It says
21.string.plus.cpp.

We should exercise long strings of more varying length rather than just
2048 characters (give or take a few). The point is to exercise the
correctness and exception safety of the reallocation algorithm. Since
our string allocates space for at least 128 characters and then each
time it needs to increase the capacity it does so by multiplying it by
1.618 (the golden ratio), I suggest adding few more test cases for
strings whose either initial or final length is close to the following
values: 128, 207, 334, 540, 873, 1412, 2284, and 3695.

The curly braces around individual cases in the switch statements are
only necessary when declaring a variable. Otherwise they are not needed
and should be removed since they are (IMO) detrimental to readability.

In the operators test, the comments describing the result say that -1
means less but we use NPOS. Could you change the cases to use -1 (you
can cast the value to size_t in the definition of the macro to avoid
warnings).

With these changes okay to commit.

Thanks again.
Martin

Mime
View raw message