Re: [boost] Formal Review: any

From: Mark Rodgers (mark.rodgers_at_[hidden])
Date: 2000-11-07 03:05:49

Here are my comments.

Documentation:

- Needs boost header.

- The link to lexical_cast will need to come out until 
  lexical_cast is accepted into Boost.

- The link to any.hpp at the top of the Synopsis would be broken 
  if any.hpp lives in boost/, which is where it should indeed 
  live.

- I find the documentation of the members very confusing.  The
  member signatures are indented and not in bold, so they look 
  like code snippets embedded in the text rather than headings.

any.hpp

- It might be nice to at least be able to compare anys for
  equality - equal if same type and contents equal.  An ordering
  using type_info::before might even be possible.  On the other
  hand, that puts additional constraints on ValueTypes.  Not
  sure what's best.  As it is, the void* conversion probably
  allows comparisons with somewhat surprising results.

- The conversion to void* is IMHO A Bad Thing.  Please remove it
  and replace with 

     bool empty() const { return content == 0; }

- I wonder why swap returns *this.  The normal convention is for
  swap functions to return void.

- I think you need to either specialise std::swap or provide a
  swap function in boost::, depending on your preference.  I think
  the jury is out on what is the correct thing to do for non-
  templates (I think probably the former).

- There are two distinct bugs being worked around with MSVC_INCLUDE
  and MSVC_EXCLUDE.  GCC shares one of them, so it is not possible
  to make any work for GCC just by defining COMPILING_MSVC (which
  you should anyway integrate into config.hpp as noted in future
  directions).  By commenting out the MSVC_INCLUDE on the copy
  constructor and assignment operator, and by defining COMPILING_MSVC
  since GCC shares the inability to write object.memfun<foo>(), I
  was able to make this work with GCC. 

- I wonder if it would be a good idea to provide a pair of to_ptr
  functions for MSVC's and GCC's benefit?

    template <typename ValueType>
    ValueType *to_ptr(any &a)
      { return a.to_ptr MSVC_EXCLUDE(<ValueType>)(
                MSVC_INCLUDE(static_cast<ValueType *>(0))); }

  and similar for const?

      boost::any a(23);
      int *p = to_ptr(a);

  seems a lot nicer than

      boost::any a(23);
      int *p = a.to_ptr(static_cast<int *>(0));

test-any.cpp:

- Please change 
    #include "any.hpp" 
  to 
    #include "boost/any.hpp"

- The test_comparisons function is very misleading since it gives
  the impression that anys can be compared sensibly.  Comparing
  0 < &typeid(void) seems particularly unuseful.

test.hpp:

- Needs copyright notice.

- I agree with Jeremy - using string as a template parameter is
  confusing.