Re: [boost] Adding a generic list manipulator

From: Terje Slettebø (tslettebo_at_[hidden])
Date: 2003-04-15 15:08:15

I intend to look more into your library, but just a few comments, now:

Regarding the use of names starting with double underscore:

 [17.4.3.1.2] Global names:

Certain sets of names and function signatures are always reserved to the
implementation:

- Each name that contains a double underscore (_ _) or begins with an
underscore followed by an upper-case
letter (2.11) is reserved to the implementation for any use.

- Each name that begins with an underscore is reserved to the implementation
for use as a name in the global namespace.

This goes for the header-guard macros, as well.

" __cdecl" - This is Microsoft-specific, so it may be a good idea to remove
(it can always be set in the project settings, instead).

Some of the headers have very odd formatting, such as array_output.hpp:

--- Start ---

   namespace boost { namespace io
   {
      template< typename FormatType, typename Outputter =
boost::io::basic_output >
      class array_output: public detail::list_output< FormatType,
array_output, Outputter >
      {
         private:
            long                       length;
            long                       offset;
         public:
            template< typename T, class OutputStream >
            inline OutputStream &                operator()
                                                 (
                                                    OutputStream & os,
                                                    const T * array
                                                 ) const

--- End ---

Is this intentional?

Some headers, such as list_output.hpp, are not stand-alone. Including it
gives several errors about undefined names. If it's not meant to be
includedby the user, it may be best to put it in a "detail" directory, to
indicate that. Even if it's just part of the implementation, it's good
practice to make headers stand-alone.

Some names don't conform to Boost's coding standard, such as
"spaceAroundList()" (instead of "space_around_list()"). This might be ok if
they're part of the implementation, not the interface, but that's not clear,
as the names are public members.

Finally, I agree with Gennadiy that the colours used in the documentation
tends to make it hard to read, when one is used to black on white, and it's
white on black. I suggest using black on white, like the other Boost
documentation.

This is meant as constructive feedback, to help you improve it. :)

Regards,

Terje