[boost] Re: [policy_ptr]

From: David Abrahams (dave_at_[hidden])
Date: 2003-10-27 15:48:31

"David B. Held" <dheld_at_[hidden]> writes:

> The latest version is in the Sandbox now under "latest".

Dave,

I realize this is somewhat trivial, but the formatting of the code
makes it hard for me to review.  For example, seeing a bunch of
functions declarared like this one after the other makes it very hard
to see what's happening:

        template <typename U, BOOST_CONVERSION_PARAMETERS>
        smart_ptr(smart_ptr<U, BOOST_CONVERSION_POLICIES> const& rhs)
        : base_type(static_cast<
            typename smart_ptr<U, BOOST_CONVERSION_POLICIES>::base_type const&
        >(rhs))
        { get_impl_ref(*this) = ownership_policy::clone(get_impl(rhs)); }

It's very dense, uses endline layout, and seems not to have much logic
in its indentation choices.  If I was really committed to preserve
density, I'd write:

        template <typename U, BOOST_CONVERSION_PARAMETERS>
        smart_ptr(smart_ptr<U, BOOST_CONVERSION_POLICIES> const& rhs)
          : base_type(static_cast<
                        typename smart_ptr<U, BOOST_CONVERSION_POLICIES>::base_type const&
                      >(rhs))
        { get_impl_ref(*this) = ownership_policy::clone(get_impl(rhs)); }

Which at least reflects scoping in the indentation.  However,
removing endline layout leads to:

        template <typename U, BOOST_CONVERSION_PARAMETERS>
        smart_ptr(smart_ptr<U, BOOST_CONVERSION_POLICIES> const& rhs)
          : base_type(
                static_cast<
                    typename smart_ptr<U, BOOST_CONVERSION_POLICIES>::base_type const&
                 >(rhs))
        { get_impl_ref(*this) = ownership_policy::clone(get_impl(rhs)); }

And personally I'd much rather see something less dense:

        template <typename U, BOOST_CONVERSION_PARAMETERS>
        smart_ptr(smart_ptr<U, BOOST_CONVERSION_POLICIES> const& rhs)
          : base_type(
              static_cast<
                  typename smart_ptr<
                      U, BOOST_CONVERSION_POLICIES
                  >::base_type const&
               >(rhs)
            )
        { 
            get_impl_ref(*this)
              = ownership_policy::clone(get_impl(rhs));
        }

Which, to my eye, suddenly makes it perfectly clear what's going on.

-- 
Dave Abrahams
Boost Consulting
www.boost-consulting.com