Re: [cpp-netlib-devel] Updated 0.4 integration branch

[Dean M. B. <mik...@gm...>]

Hi Glyn!

I apologize for taking a while to respond, I've been kept busy with
some daytime work related issues. I'm past these now and am in the
process of dealing with the backlog.

Please see my thoughts below.

On Mon, Nov 16, 2009 at 3:52 AM, Glyn Matthews <gly...@gm...> wrote:
> Hi,
>
>
> I've updated integration_0_4 and made an attempt to improve the unit tests
> therein.  The basic_message and basic_uri templates now are tested using
> std::wstring as a parameter, and this seems to work.  I have made some
> observations that I'd like to discuss with the list.
>

Thanks! That sounds like great progress to me.

> 1. boost::network::uri::uri and boost::network::uri::http::uri have a
> redundant namespace.  I understand that the directives maybe shouldn't be in
> boost::network, but a different named namespace might be better so that we
> could have boost::network::uri and boost::network::http::uri.

Do you have any proposals for that different namespace?

> 2. I had to do some workarounds with the wrappers and directives to add
> support for std::wstring.

Yeah, I think there should be a better way of doing directives in a
more generic manner like using either CRTP or PBCP correctly. Let me
get back to that at a later time, maybe once we finalize the uri
implementation.

> 3. I need clarification as to why there should be tags to identify different
> protocols.  I understand the need to specialize templates for strings and
> header_containers but I can't see why we have, e.g.
> boost::network::http::message_tags to specialize
> boost::network::http::basic_message as we do now.  this means we have to
> provide traits for struct string<http::message_tags> as well as
> string<tags::default_> which are in fact going to be identical.

The reasoning for this is so that we can have different interfaces for
the different specializations of basic_message. Although we might
probably be able to get away with a Message concept which is opaque
and can be checked/enforced by BCCL, my concern for that would be the
lack of "specialization" for writing generic algorithms that might be
able to do tag dispatching. For instance:

  template <class Tag>
  void foo(basic_message<Tag> const & msg) {
    bar(msg, Tag()); // use tag dispatching
  }

This also allows us to specialize some operations on basic_messages:

  template <class Tag, class Transformation>
  basic_message<Tag> transform(basic_message<Tag> const & msg,
transformation<Transformation> f) {
    return f(msg);
  }

Although this might work with BCCL using opaque types, it's worth a
shot. Again let me look back at cleaning up the message abstraction
and algorithms at a later time.

> 4. I don't like the name, tags::default_.  I added tags::default_string and
> tags::default_wstring, and, while they're more descriptive, I think they can
> be improved.

The intention in my brain for tags::default_ was not just for what
kind of strings to use, but also for default implementations for
example of the HTTP client and the basic_message template. The
default_ tag was meant to be the default implementation for anything
that was being shipped with the library -- and traits were dispatched
according to that tag. This allows for using compile-time flags to
switch the default typedef for boost::network::message from
`boost::network::basic_message<tags::default_>` to
`boost::network::basic_message<tags::special>` or something to that
effect.

> 5. Internally, we sometimes use namespace impl and sometimes namespace
> detail.  We should be more consistent and choose one.

I think personally there's a distinction, but I won't fight for impl.
In my mind impl:: is supposed to be just for implementations, while
detail:: can also implement helpers and utility classes, etc.;
detail::impl:: might make sense but then that would be too much
nesting even for me. ;)

I like detail and it follows the boost tradition.

> 6. The implementation of http::message and http::message uses std::string
>

Yeah, I should change that really. File Trac tickets? :) I don't
receive emails though, is there something that I should do special to
get alerts on new tickets filed and assigned to me?

> Please try and run the tests and let me know if there are any other issues.
> Dean and John, perhaps you could be able to see if the updates work with
> what you're both trying to do.
>

Is the branch cut from trunk, or was it cut from somewhere else? I'll
try to merge your changes into my branch first, then come up with a
patch set or try to merge back the stabilized version into the
integration branch.

Please expect something within the next couple of days.

Thanks again Glyn and I hope this helps!

-- 
Dean Michael Berris
blog.cplusplus-soup.com | twitter.com/mikhailberis
linkedin.com/in/mikhailberis | facebook.com/dean.berris | deanberris.com