stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: [PATCH] 23.list.iterators 23.list.cons tests with environment
Date Mon, 07 Aug 2006 23:09:06 GMT
Farid Zaripov wrote:
>   I have sent this letter to the stdcxx-dev@ at Friday, but its has not 
> reached, possible due to excess of a limit for the size of attachments.

Probably. The size limit is 100K.

> 
>   I resend this letter. The source files are here:
> http://zaripov.kiev.ua/src/
> 
> ------------------------------------------------------------------------
> ------
> 
>   The files attached is new tests of the list container with
> supporting files and changes to the existing files.

This is a big patch so I expect to have more to say about it as we
refine it but in general I like it :) There are a few relatively minor
issues that I believe should be resolved before it can be committed.

> 
> 
>   ChangeLog:
>     * sigdefs.h: New header file with definitions of helpers macros
>     to define member and non-member functions overload id's

This makes sense. We should just follow the rw_xxx.h naming convention
for all new headers (and change any existing headers that do not) so
as to avoid name clashes with headers provided by the OS.

>     * 23.containers.h: New header file with definitions of helpers
>     used in clause 23 tests.
>     * 23.containers.cpp: Ditto.
>     * 21.strings.h: (StringIds): Inherited from ContainerIds
>     from 23.containers.h. Removed definitions, which is present
>     in 23.containers.h.
>     * 21.strings.cpp: Removed definitions, which is present
>     in 23.containers.h.
>     *23.list.h: New header file with definitions of helpers used
>     in clause 23.list tests.
>     * rw_char.h: Added declarations of the functions make_char(),
>     rw_widen(), rw_expand(), rw_narrow(), rw_match() for type X.

Shouldn't these should go in alg_test instead? (X is not a character
type so I wouldn't look for functions that operate on it in a header
or source file that declares character helpers). Btw., it would be
helpful to rename X to something at least a little more meaningful,
e.g., UserValue (suggestions for better names are welcome), and move
it to a header of its own (i.e., separate from alg_test.h), say
rw_value.h.

Also, why do we need make_char(X)? If it's because we want to
exercise containers specialized on POD types I wonder if we shouldn't
instead introduce a more appropriately named class, one that doesn't
come with as much baggage as UserChar (UserTraits and UserInt). E.g.,
UserPOD, or UserStruct, or something like that.

>     * char.cpp: Added definitions of the functions make_char(),
>     rw_widen(), rw_expand(), rw_narrow(), rw_match() for type X.

Same as above.

>     (_rw_expand): Added support of the type X.
>     (_rw_fmtstringv): Added support of the type X.

X is already handled -- see _rw_fmtxarray() in alg_test.cpp. Btw.,
with the proliferation of user-defined data types in the test suite
it might make sense to "derive" all of them from the same "base" POD
type so that they can all be formatted using the same function.

I'm using the term "derived" and "base" in a loose sense and not in
the C++ sense of the word, since UserChar certainly cannot be derived
from anything (it must be a POD type). But UserChar could contain
a member of UserStruct:

     struct UserStruct {   // POD type
         // data members
     };

     struct UserChar {   // POD type, same layout as UserStruct
         UserStruct data_;
     };

     struct UserValue {   // not POD type, same layout as UserStruct
         UserStruct data_;
         UserValue ();    // ctor
         // ...other members...
         // ...static data members...
      };

Let me know what you think about this approach (it would require
non-trivial changes to existing code but it would reduce code
duplication in the test driver and simplify the maintainability
of the rest of the test suite).

>     * 21.string.iterators.cpp (test_iterators): Fixed incorrect use
>     of RW_ASSERT().
>     * 23.list.iterators.cpp: New test exercising the list members:
>     begin(), end(), rbegin(), rend(), front(), back(), get_allocator().
>     *23.list.cons.cpp: New test exercising list constructors and
>     list<>::operator=.

I'll comment on these separately.

Martin

Mime
View raw message