[Xorp-hackers] Async helper classes

Steven Simpson Fri, 03 Jun 2011 04:07:16 -0700

Hi again,

I started considering some helper classes that could be generated with the callback typedef that an async method implementation uses to respond. Here are the declarations for the common/0.1/get_target_name XRL, which has no in-args and one string out-arg:

    virtual XrlCmdError common_0_1_get_target_name(
	// Output values,
	string&	name) = 0;
#ifdef XORP_ENABLE_ASYNC_SERVER
    typedef
    XorpCallback2<void, const XrlCmdError &,
	const string*>::RefPtr
    Common01GetTargetNameCB;


    virtual void async_common_0_1_get_target_name
       (
	Common01GetTargetNameCB);

So there's the async callback type *CB in use. It requires a pointer to a string to pass the 'name' out-arg.

To implement the async_* method, you'd have to do (at some point):

Common01GetTargetNameCB cb;
string result = "my name";
cb->dispatch(XrlCmdError::OKAY(), &result);

It might seem a little odd/inconvenient that you have to store the result in a variable (i.e. no literals), but this is to conform to the last patch I sent, which declares all the out-arguments as pointers, so they can be NULL when reporting an error, e.g.:

cb->dispatch(XrlCmdError::COMMAND_FAILED("miserably"), NULL);

Really, you ought to have two methods, one taking only the out-arguments, and one taking only an error code (unless there's a case where you can send a non-OKAY error code with out-arguments, but my understanding is that that doesn't happen, or if it did, the arguments are ignored - correct me if I'm wrong).

This helper class splits the callback into two methods:

    class Common01GetTargetNameResponse {
        Common01GetTargetNameCB cb;

    public:
        Common01GetTargetNameResponse(const Common01GetTargetNameCB& cb)
          : cb(cb) { }

        void fail(const XrlCmdError &e) { cb->dispatch(e, NULL); }

        void respond(const string& arg_name) {
            cb->dispatch(XrlCmdError::OKAY(),
                         &arg_name);
        }
    };

So now you can send a normal response with:

Common01GetTargetNameCB cb;
Common01GetTargetNameResponse rsp(cb);
rsp.respond("my name"); // OKAY implied

Furthermore, you can send an error with just:

Common01GetTargetNameCB cb;
Common01GetTargetNameResponse rsp(cb);
rsp.fail(XrlCmdError::COMMAND_FAILED("miserably")); // no NULLs

Here's a second helper derived from the first:

    struct Common01GetTargetNameResponse2 : public Common01GetTargetNameResponse {
        struct args_str {
            string name;
        };

    private:
        args_str args;

    public:
        Common01GetTargetNameResponse2(const Common01GetTargetNameCB& cb)
          : Common01GetTargetNameResponse(cb) { }

        void respond() {
            Common01GetTargetNameResponse::
            respond(args.name);
        }

        args_str* operator ->() {
            return &args;
        }
    };

Failures are handled as before, but the *Response2 class holds your values for you:

Common01GetTargetNameCB cb;
Common01GetTargetNameResponse2 rsp(cb);
rsp->name = "my name";
rsp.respond();

I guess that would be convenient for some cases too, but I'm less sure.

Anyway, what do you think? Worthwhile?

Any naming suggestions? I'm not too pleased with respond() vs fail(), or *Response vs *Response2.

Should the async_* function take a *Response class as an argument instead of *CB? It would be an intransparent interface change, but only for those who have already written an async implementation. (Anyone else but me written one yet?) Maybe there's a way to write another async_* function which the current one could call, then you would override whichever suited your needs…

// The new async_* taking a helper:
void
XrlFinderclientTargetBase::async_common_0_1_get_target_name(
	const Common01GetTargetNameResponse &rsp)
{
    string rarg_name;
    XrlCmdError e = common_0_1_get_target_name(rarg_name);
    if (e.isOK())
        rsp.respond(rarg_name);
    else
        rsp.fail(e);
}

// The old async_* signature, chaining onto the new.
void
XrlFinderclientTargetBase::async_common_0_1_get_target_name(
	Common01GetTargetNameCB c_b)
{
    Common01GetTargetNameResponse rsp(cb);
    async_common_0_1_get_target_name(rsp);
}

But is all this chaining overdoing it? And more so if *Response2 were a further link in the chain?

I've just been investigating binary sizes. As I hoped, the non-virtual inline functions above would have no impact at all when unused — indeed, they don't seem to have any impact even when used! They must get optimized away altogether. It will likely be a different story if used in the async_* chain.

So, in summary:

Can you suggest better names for the classes and methods?
Use *Response in the chain:

by inserting another async_* step,
by inserting another step, and deprecating the old one with a view to removing it,
by just replacing the old one now, and requiring everyone who's used it (hopefully very few) to change,
not at all?

Cheers,

Steven

--

_______________________________________________
Xorp-hackers mailing list
[email protected]
http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/xorp-hackers

[Xorp-hackers] Async helper classes

Reply via email to