stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: svn commit: r395448 - in /incubator/stdcxx/trunk/tests: include/21.strings.h src/21.strings.cpp
Date Fri, 21 Apr 2006 21:40:07 GMT
Anton Pevtsov wrote:
> <>Martin Sebor wrote:
>  > I added these to the 21.strings.h header and modified the append and
>  > assign tests but I didn't have time to see if the changes are enough
>  > to accommodate the replace test.
> 
> Thanks. These changes are enough. The patch to the replace test is
> attached. Also there are changes I made to 21.strings.h and .cpp. I just 
> added new format function which accepts additional parameter -
> the method name.

I made a few changes and applied the result here:
http://svn.apache.org/viewcvs.cgi?rev=396006&view=rev

> 
> I have a question: could you explain why you have moved "which" member
> to the TestCase structure, please?
> It looks like it is impossible to have one test cases array for two
> replace overloads now.

Whoops. I had missed that some of the test cases arrays are being
used to exercise two member functions, sorry. Let me rethink this.
(I thought you had suggested something like it but after rereading
your email I see you were actually talking about the Test struct,
not TestCase).

> 
> And I' suspect a problem with
>     int StringMembers::
>     opt_memfun_disabled [StringMembers::member_functions];
> 
> the StringMembers::member_functions is equal to 10, but the replace_...
> enumeration members have values starting from 8. Is it correct?

Yes, you're right. It was caused by the enumerators for the member
function counts. I fixed it by moving those into an enum of their
own (see http://svn.apache.org/viewcvs.cgi?rev=395745&view=rev).

> I implemented the workaround in the replace test, but it is an ugly
> patch.

Since I fixed the cause of the problem I was going to remove this
"ugly" change but I couldn't find it...

> 
> But I think that there is no need to have separate enum members for each
> method. Instead we may have enumeration elements describing method 
> signature
> (there are about 20 different signatures), i.e:
[...]
> The format function will contain string for each enumeration member (the
> format doesn't depend on method, only on parameters).

That sounds like a good idea. On second thought, though, I wonder
if it might perhaps still be useful to have a unique numeric id
(or the ability to create one on the fly) for each overload.

So the way to go might be to have one enum for just the signatures
as you suggest above and another for each group of overloads (i.e.,
something like enum MemberFunction { append, assign, insert, ... };

> I think there is 
> no need in special formatting directive at this time -
> the format and format_hdr (the same to format but only parameters
> declaration) functions will do all work.
> 
> Of course, in this case it will be impossible to test several methods in
> one test function. But I think this doesn't make sense.
> 
> What do you think about this?
> 
> 
> Also I suggest to move the MemFun structure to the 21.strings.h header.
> This allow us to move run_test and top-level test functions to
> 21.strings.cpp and make them common for all tests.
> Is there any constraint that I missed and which doesn't allow us do
> this?

I don't think so. I'm still mulling over about how best to do this.
I'm wondering how to move as much boilerplate code from these tests
into the driver and if it might be easier to change the format of
all the tests to use the virtual function call mechanism (the way
we implemented 25.merge.cpp).

Martin

Mime
View raw message