stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: test for 21.string::erase
Date Wed, 08 Mar 2006 02:16:47 GMT
Anton Pevtsov wrote:
> The attached file contains the test for 21.string::erase.

Looks a lot better than the original! I wonder if we could improve
it though (actually, I have a pretty good idea we could and how :)
See my comments below.

> Here I used classes from the rw_char.h header. I plan to extend this
> test with UserChar, but there is a problem with the formatting output of
> the strings with UserChar's.
> Does the %{#*S} directive support the strings of UserChars'?

Not yet. I'll need to think about a generic and convenient interface.

> 
> With best wishes,
> Anton Pevtsov
> 
> 
> 
> ------------------------------------------------------------------------
> 
> /***************************************************************************
>  *
>  * erase.cpp - string test exercising [lib.string::erase]
>  *             and the lifetime of iterators and references

Does the new test actually do all that? If not we will need to
remember to add a new test to exercise it (maybe open an issue).

[...]
> template <class charT>
> void widen (charT *buf, const char *str, const std::size_t str_len)

I added rw_widen() to <rw_char.h> as a replacement for this so you
can remove this template.

[...]
> template <class charT, class Traits>
> void test_erase (charT, Traits, MemFun* pfid,
>                  int         line,
>                  const char *str,
>                  std::size_t str_len,
>                  int         pos,
>                  int         cnt,
>                  const char *res,
>                  std::size_t res_len,
>                  bool        should_throw)
[...]
>     static charT wres [long_string_len];
>     if (0 != res)
>         widen (wres, res, res_len);

rw_widen() handles NULL source strings so there's no need for the
check here.

[...]
> template <class charT, class Traits>
> void test_erase_iters (charT, Traits, MemFun* pfid,
>                        int         line,
>                        const char *str,
>                        std::size_t str_len,
>                        int         first,
>                        int         last,
>                        const char *res,
>                        std::size_t res_len,
>                        bool        should_throw)
> {
>     typedef std::allocator<charT> Allocator;
>     typedef std::basic_string<charT, Traits, Allocator> TestString;
> 
>     _RWSTD_UNUSED (should_throw);

If this function doesn't throw we should eliminate the should_throw
argument.

OTOH, I would expect this function to be almost the same as test_erase
except for the fact that it deals with iterators and not integers, of
course. Would it make sense to collapse both functions into one since
they share so much code? In fact, because of the small number of tested
member functions and their similarity I think this test lends itself
to the same implementation technique as the 25.set.*.cpp tests (i.e.,
a non-template base with a derived template parametrized on charT,
Traits, and Allocator. (I chose this approach in stringbuf.virtuals
because of the large number of member functions that needed to be
exercised and the differences in their signatures).

Wouldn't implementing this test along the lines I suggest be better?

Also, it seems to me that just one set of hardcoded test cases below
could be used to exercise both functions instead of hardcoding each
set twice with ever-so-slightly different arguments.

I.e., given the five overloads of erase

     1. erase();
     2. erase(size_type);
     3. erase(size_type, size_type);
     4. erase(iterator);
     5. erase(iterator, iterator);

we can invoke the test function using the following:

     test_erase (int which) {
         TEST (string, offset, count, result, exception);
         ...
      }

with

     string is the same in both cases
     1 == which: (offset < 0 && count < 0)
     2 == which: (0 <= offset && count < 0)
     3 == which: (0 <= offset && 0 <= count)
     4 == which: (0 <= offset && count < 0 && !exception)
     5 == which: (0 <= offset && 0 <= count && !exception)

If (4 == which || 5 == which) the test function returns without any
effects (i.e., w/o invoking erase or even constructing the iterators)
when the following is true:

     (   sizeof (string) <= offset
      || sizeof (string) < offset + count
      || exception == true)

Otherwise it constructs the iterator(s) from offset and (optionally
count) and invokes overload 4 or 5 of erase() and verifies that the
result is the same as in cases 2 or 3.

Let me know if this makes sense.
Martin

Mime
View raw message