Re: [PATCH] Doxygen documentation for slist

[Jonathan Wakely <cow at compsoc dot man dot ac dot uk>]

On Wed, Jan 26, 2005 at 01:08:41AM +0100, Lorenz Minder wrote:

> Hi,
> 
> I wrote up some documentation for __gnu_cxx::slist, borrowing a lot from
> what was there already for std::list.

Excellent!

> I'm not an experienced writer for technical documentation, nor is
> english my mother tongue, so comments and improvements are highly
> welcome.

I've made a few suggestions below, I hope they're useful.

> I also have some questions:
> 
> - Do I need copyright assignment forms for that kind of change? If so,
>   who do I have to ask to get them?
> 
> - Should I wait until 4.0 is out to submit such changes? (It's not
>   exactly a bug fix, but OTOH these changes are very low-risk, and they
>   affect also the web pages.)

I can't say for sure, but I expect these will be OK for 4.0

>    /**
> +   *  @brief A container with linear time access to elements, and constant
> +   *  time for for insertion and deletion @e after an iterator.
              ^^^^^^^
> +   *  An slist is a @e singly @e linked list, as opposed to the
> +   *  std::list container, which is doubly linked. This means that it is
> +   *  possible to iterate over an slist only in forward direction,
                                                  ^ in /the/ forward
> +   *  whereas lists allow for bidirectional iterators.
                                               ^iteration
> +   *
> +   *  Note that insertion and deletion with the insert() and erase()
> +   *  operations are slow for slists, since they require a linear
> +   *  search. The insert_after() and erase_after() methods allow for
> +   *  insertion and deletion in constant time anywhere in the list.
> +   *  Slists don't have random access iterators, and elements can't be
> +   *  accessed via subscripts ([]).

         I'm not sure this last sentence is very useful. Most
         containers do not support random access iterators or
         operator[], in particular std::list doesn't.

> +   *
> +   *  The advantage of %slist over %std::list is that per stored
> +   *  element, one pointer can be saved, and it therefore uses less
> +   *  memory. Usually, %std::list is more recommendable, since it is
> +   *  standard and more portable.

         Maybe reword this to say "Usually std::list is recommended
         since it is standard and more portable."

> +      /**
> +       *  @brief  The default constructor creates an empty slist.
                                                              ^ %slist
> +       */
>        explicit
>        slist(const allocator_type& __a = allocator_type())
>        : _Base(__a) {}
>  

> -      // We don't need any dispatching tricks here, because
> -      // _M_insert_after_range already does them.
> +      /**
> +       *  @brief  Builds a %list from a range.
> +       *  @param  first  An input iterator.
> +       *  @param  last  An input iterator.

I think this should be formatted like so:
> +       *  @brief  Builds a %list from a range.
> +       *  @param  first  An input iterator.
> +       *  @param  last   An input iterator.

So that the parameter descriptions are lined up. There are a number of
places that could be tidied up like this but the std::list docs aren't
all formatted like that, so it's not that important.

> +       *  It only invalidates iterators/references to the element being
> +       *  removed.  The user is also cautioned that this function only
> +       *  erases the elements, and that if the elements themselves are
> +       *  pointers, the pointed-to memory is not touched in any way.
> +       *  Managing the pointer is the user's responsibilty.

This is true of all containers, does it need to be said?

> +      /**
> +       *  @brief  Sort the elements.
> +       *
> +       *  Sorts the elements of this list in NlogN time.  Equivalent
> +       *  elements remain in list order.

i.e. "sort is stable"

Hope that helps,

jon

-- 
"Convictions cause convicts"
	- Mal2