cpp-netlib-devel

[cpp-netlib-devel] About Handler::log()

[Emre T. <emr...@gm...>]

Hi folks,

In file boost/network/protocol/http/server/sync_connection.http (in sync_connection::start), line 57, there is a call to Handler::log(string).
1. This conflicts with the documentation, where there is no hello_world::log() method implemented in the simple http server example.
2. Do we really need to keep it that way, i.e., we can make boost::system::system_error an extra argument to the handler function (like asio handlers). By this way, it would even be possible to implement a handler with a free standing function and pass the http::server a ptr to function instead of a functor, for simple cases.

What you think?

emre

Re: [cpp-netlib-devel] About Handler::log()

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

Hi Emre,

On Wed, Feb 23, 2011 at 6:22 AM, Emre Türkay <emr...@gm...> wrote:
> Hi folks,
>
> In file boost/network/protocol/http/server/sync_connection.http (in sync_connection::start), line 57, there is a call to Handler::log(string).
> 1. This conflicts with the documentation, where there is no hello_world::log() method implemented in the simple http server example.
> 2. Do we really need to keep it that way, i.e., we can make boost::system::system_error an extra argument to the handler function (like asio handlers). By this way, it would even be possible to implement a handler with a free standing function and pass the http::server a ptr to function instead of a functor, for simple cases.
>
> What you think?
>

I like #2 -- do you have a pull request? :)

-- 
Dean Michael Berris
about.me/deanberris

Re: [cpp-netlib-devel] About Handler::log()

[Emre T. <emr...@gm...>]

Hi Dean,

On Feb 24, 2011, at 7:01 AM, Dean Michael Berris wrote:

> Hi Emre,
> 
> On Wed, Feb 23, 2011 at 6:22 AM, Emre Türkay <emr...@gm...> wrote:
>> Hi folks,
>> 
>> In file boost/network/protocol/http/server/sync_connection.http (in sync_connection::start), line 57, there is a call to Handler::log(string).
>> 1. This conflicts with the documentation, where there is no hello_world::log() method implemented in the simple http server example.
>> 2. Do we really need to keep it that way, i.e., we can make boost::system::system_error an extra argument to the handler function (like asio handlers). By this way, it would even be possible to implement a handler with a free standing function and pass the http::server a ptr to function instead of a functor, for simple cases.
>> 
>> What you think?
>> 
> 
> I like #2 -- do you have a pull request? :)

No, I wanted to ask before diving in. I'm still reading the docs ;) I'll try if I can make it. 

> 
> -- 
> Dean Michael Berris
> about.me/deanberris

Re: [cpp-netlib-devel] About Handler::log()

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

On Thu, Feb 24, 2011 at 3:55 PM, Emre Türkay <emr...@gm...> wrote:
> Hi Dean,
>
> On Feb 24, 2011, at 7:01 AM, Dean Michael Berris wrote:
>
>> Hi Emre,
>>
>> On Wed, Feb 23, 2011 at 6:22 AM, Emre Türkay <emr...@gm...> wrote:
>>> Hi folks,
>>>
>>> In file boost/network/protocol/http/server/sync_connection.http (in sync_connection::start), line 57, there is a call to Handler::log(string).
>>> 1. This conflicts with the documentation, where there is no hello_world::log() method implemented in the simple http server example.
>>> 2. Do we really need to keep it that way, i.e., we can make boost::system::system_error an extra argument to the handler function (like asio handlers). By this way, it would even be possible to implement a handler with a free standing function and pass the http::server a ptr to function instead of a functor, for simple cases.
>>>
>>> What you think?
>>>
>>
>> I like #2 -- do you have a pull request? :)
>
> No, I wanted to ask before diving in. I'm still reading the docs ;) I'll try if I can make it.
>

Cool, no worries. :)

I suggest looking at the cases where the log function is called -- I
think calling the handler with an extra error argument is a little
confusing. I like the approach of providing an error handling function
to the constructor of the HTTP server, and if one isn't provided a
default error handler is provided that optionally prints information
to standard error. There was a suggestion to use Boost.Log but I don't
see how this could be done with Boost.Log not yet being in the
releases (last I checked).

This should be easy to do and I would really appreciate it if you can
give that a shot -- as I have very limited time at the moment to be
able to work on the library.

Have a good one and I look forward to your pull requests!

-- 
Dean Michael Berris
about.me/deanberris

Re: [cpp-netlib-devel] About Handler::log()

[Emre T. <emr...@gm...>]

Hi Dean,

We can provide various methods to register callback, actually maybe all of them. Using boost::function (with or without a system::error argument) + a class with log() function + two different handlers for data & error. But, to provide this flexibility we will need to have a "virtual" overhead somewhere deep in the implementation. I'll give it a try for this, after that we can think about it further.

Take care,

emre

On Feb 24, 2011, at 10:02 AM, Dean Michael Berris wrote:

> On Thu, Feb 24, 2011 at 3:55 PM, Emre Türkay <emr...@gm...> wrote:
>> Hi Dean,
>> 
>> On Feb 24, 2011, at 7:01 AM, Dean Michael Berris wrote:
>> 
>>> Hi Emre,
>>> 
>>> On Wed, Feb 23, 2011 at 6:22 AM, Emre Türkay <emr...@gm...> wrote:
>>>> Hi folks,
>>>> 
>>>> In file boost/network/protocol/http/server/sync_connection.http (in sync_connection::start), line 57, there is a call to Handler::log(string).
>>>> 1. This conflicts with the documentation, where there is no hello_world::log() method implemented in the simple http server example.
>>>> 2. Do we really need to keep it that way, i.e., we can make boost::system::system_error an extra argument to the handler function (like asio handlers). By this way, it would even be possible to implement a handler with a free standing function and pass the http::server a ptr to function instead of a functor, for simple cases.
>>>> 
>>>> What you think?
>>>> 
>>> 
>>> I like #2 -- do you have a pull request? :)
>> 
>> No, I wanted to ask before diving in. I'm still reading the docs ;) I'll try if I can make it.
>> 
> 
> Cool, no worries. :)
> 
> I suggest looking at the cases where the log function is called -- I
> think calling the handler with an extra error argument is a little
> confusing. I like the approach of providing an error handling function
> to the constructor of the HTTP server, and if one isn't provided a
> default error handler is provided that optionally prints information
> to standard error. There was a suggestion to use Boost.Log but I don't
> see how this could be done with Boost.Log not yet being in the
> releases (last I checked).
> 
> This should be easy to do and I would really appreciate it if you can
> give that a shot -- as I have very limited time at the moment to be
> able to work on the library.
> 
> Have a good one and I look forward to your pull requests!
> 
> -- 
> Dean Michael Berris
> about.me/deanberris

Re: [cpp-netlib-devel] About Handler::log()

[Emre T. <emr...@gm...>]

Hi Dean.

What do you say about this interface? Take a look at the various possible handlers.

#include <iostream>

// Fundamental meta-functions.

struct my_specific_tag { };

template <typename T>
struct has_a_tag
{
private:
    typedef char true_type;
    struct false_type { true_type _[2]; };

    template <typename U>
    static true_type has_tag_checker(typename U::tag*);
    template <typename U>
    static false_type has_tag_checker(...);

public:
    static const bool value =
        sizeof(has_tag_checker<T>(0)) == sizeof(true_type);
};

template <typename T>
struct has_not_a_tag
{
    static const bool value = !has_a_tag<T>::value;
};

template <bool Cond, class T = void>
struct enable_if
{
    typedef T type;
};

template <class T>
struct enable_if<false, T>
{
};

// A sample framework:

#include <boost/function.hpp>
#include <boost/bind.hpp>
#include <string>

struct data_type
{
    data_type(const std::string& val)
        : value_(val)
    {
    }

    friend std::ostream& operator<<(std::ostream& os, const data_type& data)
    {
        return os << data.value_;
    }

private:
    std::string value_;
};

struct error_type
{
    error_type(const std::string& val)
        : value_(val)
    {
    }

    friend std::ostream& operator<<(std::ostream& os, const error_type& error)
    {
        return os << error.value_;
    }

private:
    std::string value_;
};

struct framework
{
    framework()
        : data_("No data"),
          error_("No error")
    {
    }

    template <typename T>
    typename enable_if<has_a_tag<T>::value>::type register_handler(T t)
    {
        priv_register_handler(t, typename T::tag());
    }

    template <typename T>
    typename enable_if<has_not_a_tag<T>::value>::type register_handler(T t)
    {
        data_callback_ = boost::bind<void>(t, _1, error_);
        error_callback_ = boost::bind<void>(t, data_, _1);
    }

    template <typename T1, typename T2>
    void register_handler(T1 f1, T2 f2)
    {
        data_callback_ = f1;
        error_callback_ = f2;
    }

    void call(data_type data)
    {
        data_callback_(data);
    }

    void call(error_type error)
    {
        error_callback_(error);
    }

private:
    template <typename T, typename U>
    void priv_register_handler(T t, U)
    {
        data_callback_ = boost::bind<void>(t, _1, error_);
        error_callback_ = boost::bind<void>(t, data_, _1);
    }

    template <typename T>
    void priv_register_handler(T t, my_specific_tag)
    {
        data_callback_ = boost::bind(&T::handle_data, &t, _1);
        error_callback_ = boost::bind(&T::handle_error, &t, _1);
    }

private:
    boost::function<void(data_type)> data_callback_;
    boost::function<void(error_type)> error_callback_;

    data_type data_;
    error_type error_;
};

// Handlers:

void handler1(data_type data, error_type error)
{
    std::cout << "handler1("
              << "data: '" << data << "', "
              << "error: '" << error << "')" << std::endl;
}

struct handler2
{
    void operator()(data_type data, error_type error)
    {
        std::cout << "handler2("
                  << "data: '" << data << "', "
                  << "error: '" << error << "')" << std::endl;
    }
};

struct handler3
{
    typedef my_specific_tag tag;

    void handle_data(data_type data)
    {
        std::cout << "handler3(data: '" << data << "')" << std::endl;
    }

    void handle_error(error_type error)
    {
        std::cout << "handler3(error: '" << error << "')" << std::endl;
    }
};

void handler4_data(data_type data)
{
    std::cout << "handler4(data: '" << data << "')" << std::endl;
}

void handler4_error(error_type error)
{
    std::cout << "handler4(error: '" << error << "')" << std::endl;
}

int main()
{
    data_type data("data");
    error_type error("error");

    framework frm;

    frm.register_handler(handler1);
    frm.call(data);
    frm.call(error);

    frm.register_handler(handler2());
    frm.call(data);
    frm.call(error);

    frm.register_handler(handler3());
    frm.call(data);
    frm.call(error);

    frm.register_handler(handler4_data, handler4_error);
    frm.call(data);
    frm.call(error);
}


On Feb 24, 2011, at 10:02 AM, Dean Michael Berris wrote:

> On Thu, Feb 24, 2011 at 3:55 PM, Emre Türkay <emr...@gm...> wrote:
>> Hi Dean,
>> 
>> On Feb 24, 2011, at 7:01 AM, Dean Michael Berris wrote:
>> 
>>> Hi Emre,
>>> 
>>> On Wed, Feb 23, 2011 at 6:22 AM, Emre Türkay <emr...@gm...> wrote:
>>>> Hi folks,
>>>> 
>>>> In file boost/network/protocol/http/server/sync_connection.http (in sync_connection::start), line 57, there is a call to Handler::log(string).
>>>> 1. This conflicts with the documentation, where there is no hello_world::log() method implemented in the simple http server example.
>>>> 2. Do we really need to keep it that way, i.e., we can make boost::system::system_error an extra argument to the handler function (like asio handlers). By this way, it would even be possible to implement a handler with a free standing function and pass the http::server a ptr to function instead of a functor, for simple cases.
>>>> 
>>>> What you think?
>>>> 
>>> 
>>> I like #2 -- do you have a pull request? :)
>> 
>> No, I wanted to ask before diving in. I'm still reading the docs ;) I'll try if I can make it.
>> 
> 
> Cool, no worries. :)
> 
> I suggest looking at the cases where the log function is called -- I
> think calling the handler with an extra error argument is a little
> confusing. I like the approach of providing an error handling function
> to the constructor of the HTTP server, and if one isn't provided a
> default error handler is provided that optionally prints information
> to standard error. There was a suggestion to use Boost.Log but I don't
> see how this could be done with Boost.Log not yet being in the
> releases (last I checked).
> 
> This should be easy to do and I would really appreciate it if you can
> give that a shot -- as I have very limited time at the moment to be
> able to work on the library.
> 
> Have a good one and I look forward to your pull requests!
> 
> -- 
> Dean Michael Berris
> about.me/deanberris

Re: [cpp-netlib-devel] About Handler::log()

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

On Sat, Mar 19, 2011 at 9:22 PM, Emre Türkay <emr...@gm...> wrote:
> Hi Dean.
>
> What do you say about this interface? Take a look at the various possible handlers.
>

Hmmm... I don't understand what you're trying to accomplish here. Care
to elaborate? If it's just a matter of providing an error handler,
that should be really *easy* and not this complicated. I appreciate
the code, but I don't know what problem you're trying to solve here.

-- 
Dean Michael Berris
http://about.me/deanberris

Re: [cpp-netlib-devel] About Handler::log()

[Emre T. <emr...@gm...>]

Hi Dean,

While looking at the documentation I see that the handler is originally designed to be a function object.
That's nice, so the application developer can:

- Provide a function if the case is a simple one, or
- If the implementation needs state information, provide a class with a function call operator overloaded.

However, the call to the Handler::log() brakes that simplicity, the handler is forced to be a class.
Below are the possible solutions to this problem, with each having their own pro & cons (mostly taste related).

1. Provide an extra error argument to the function.
2. Handler::log() assumption is fine, let's force the implementation to be a class with specific
     handle_data() and handle_error() functions.
3. Provide 2 different handler functions, one for data and one for error.

So I have provided an interface to handle all. I think being easy is nothing to do with this.
Here, the interface to the application developer is easy, and the implementation is a little
bit complicated.

By the way there is no need of the template argument Handler in this implementation.

On Mar 20, 2011, at 12:04 PM, Dean Michael Berris wrote:

> On Sat, Mar 19, 2011 at 9:22 PM, Emre Türkay <emr...@gm...> wrote:
>> Hi Dean.
>> 
>> What do you say about this interface? Take a look at the various possible handlers.
>> 
> 
> Hmmm... I don't understand what you're trying to accomplish here. Care
> to elaborate? If it's just a matter of providing an error handler,
> that should be really *easy* and not this complicated. I appreciate
> the code, but I don't know what problem you're trying to solve here.
> 
> -- 
> Dean Michael Berris
> http://about.me/deanberris

Re: [cpp-netlib-devel] About Handler::log()

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

On Mon, Mar 21, 2011 at 2:44 AM, Emre Türkay <emr...@gm...> wrote:
> Hi Dean,
>
> While looking at the documentation I see that the handler is originally designed to be a function object.

Yep.

> That's nice, so the application developer can:
>
> - Provide a function if the case is a simple one, or
> - If the implementation needs state information, provide a class with a function call operator overloaded.
>

Well, not really...

> However, the call to the Handler::log() brakes that simplicity, the handler is forced to be a class.
> Below are the possible solutions to this problem, with each having their own pro & cons (mostly taste related).
>
> 1. Provide an extra error argument to the function.

This is confusing because it makes the the function handle two different things.

> 2. Handler::log() assumption is fine, let's force the implementation to be a class with specific
>     handle_data() and handle_error() functions.

Not really. The Handler::log(...) assumption is meant to be a contract
between the handler and the server implementation.

> 3. Provide 2 different handler functions, one for data and one for error.
>

This can be done without changing the way things are done as it is.

> So I have provided an interface to handle all. I think being easy is nothing to do with this.
> Here, the interface to the application developer is easy, and the implementation is a little
> bit complicated.
>

Unfortunately, here's the problem with trying to support all three of
the things above:

1. There's no "one way" to do it. Because of that you end up with
complications that are really unnecessary especially since everything
you want to do can be done with the current implementation already.

2. I don't still understand why you would want to have just a function
when you still have to worry about synchronization and connection
state in the handler implementation. The reason why I'm forcing the
server handler to be a type/class is so that data touched by the
handlers are actually encapsulated in the handler.

3. There's a way to adapt any function in a type that has the `log`
function so that it can be used as a handler to the server. I can give
an example when I get more time to do it.

> By the way there is no need of the template argument Handler in this implementation.
>

Which is bad because your compiler cannot optimize the parts where the
handler is being called if you just hold a boost::function to the
handler. ;)

I would be more interested in a service framework which would allow
anyone to map things like 'GET /' or 'POST /' to actual handlers. That
can be built on top of the existing interface and if this is what
you're looking at doing, I can share some thoughts with you on how to
do that instead. In the meantime a pull request to the 0.9-devel
branch to port all Handler::log(string_type const &) to become
Handler::log(boost::system::error_code const &, string_type const &)
would be most appreciated to minimize the interface breakage.

I also would like to see if you can find a way to remove the
Handler::log requirement by making the error function handler be made
part of the server constructor instead -- and maybe logic to use the
Handler::log if it's defined, or use the error handler function
defined in the server constructor. What do you think about that? :D

Thanks and HTH

> On Mar 20, 2011, at 12:04 PM, Dean Michael Berris wrote:
>
>> On Sat, Mar 19, 2011 at 9:22 PM, Emre Türkay <emr...@gm...> wrote:
>>> Hi Dean.
>>>
>>> What do you say about this interface? Take a look at the various possible handlers.
>>>
>>
>> Hmmm... I don't understand what you're trying to accomplish here. Care
>> to elaborate? If it's just a matter of providing an error handler,
>> that should be really *easy* and not this complicated. I appreciate
>> the code, but I don't know what problem you're trying to solve here.
>>
>> --
>> Dean Michael Berris
>> http://about.me/deanberris
>
>



-- 
Dean Michael Berris
http://about.me/deanberris