incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: Tests for string non-members - operators
Date Fri, 26 May 2006 18:29:52 GMT
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