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: test for lib.string.iterators
Date Fri, 16 Jun 2006 20:18:00 GMT
Anton Pevtsov wrote:
> The test exercising the string methods begin, end, rbegin, rend, c_str,
> data and get_allocator is here:
> http://people.apache.org/~antonp/stdcxx06162006/

Thanks! Just a few comments:

If all the hardcoded pairs of strings are the same (as the
c_str_void_test_cases array looks to be) it would be good to avoid
having to duplicate each by hand. You can use the macro to copy its
argument into both members of the struct.

It would be nice to change the series of if statements into either
a block of if/else statements or (if possible), into a single case
and switch statement (could we move some of them into the main case
and switch statement? e.g., checking the iterator relationships?)

When splitting an expression such as a conditional across multiple
lines the style convention is to start the second and each subsequent
line with the operator, justifying operands on preceding and/or
subsequent lines accordingly. E.g., like so

     if (   func.which_ == StringIds::begin_void    // #1
         || func.which_ == StringIds::end_void)

and not like this:

     if (func.which_ == StringIds::begin_void       // #2
         || func.which_ == StringIds::end_void)

or like this:

      if (func.which_ == StringIds::begin_void ||   // #3
          func.which_ == StringIds::end_void)

The rationale for going with #1 is readability: putting the
operator first rather than last (#3) makes it easier to see
the operation being evaluated. Justifying both of the operands
rather to the same column makes it easier to spot them when
the expression involves several non-trivial subexpressions.

With these changes (ifpossible) please go ahead and check it
in.

Martin

Mime
View raw message