Remove CROW_ROUTE macro. about crow HOT 19 OPEN

Any implementation idea?

from crow.

If we changed the route function signature to:

template <typename U>
void route(U route, U::callbackType callback);

Then wouldn't the following work?

app.route("/hello/<int>"_route, [](int count){ ... });

Where _route is a custom string literal that returns an object of similar type to

crow::black_magic::get_parameter_tag(url)

Alternatively we could also do something like:

app.route(crow::r("/hello/<int>"), [](int count){ ... });

from crow.

app.route(crow::r("/hello/<int>"), [](int count){ ... });

I like this one. No oddness to the syntax, clear and explicit.

from crow.

The problem was that return type from get_parameter_tag or crow::r should be always same type because the argument type is the same. C++ don't support function overloading of different return types.

I bypassed this issue by returning int from get_parameter_tag and reconstruct template type list from the integer. The integer should be computed at compile time (because it is used as template argument), so it cannot be given as a function argument (constexpr still have many restrictions).

But I did some researches and I think I found some clues for removing macros by using the type erasure technique. I will show you soon if I succeeded.

from crow.

Ah gotcha. I'm not a C++ meta-programming expert :)

from crow.

Perhaps rather than writing the type in the url, you could just define a placeholder and take the argument types from the function, e.g.

app.route("/add/__/__", [](int a, int b) { ... });

Flask requires typenames in the url because python's function aren't strongly typed. If the type is specified once, you don't have to check for consistency at compile time. Ideally, you'd still be able to check the number of arguments matched at compile time, but given that you can't eliminate all runtime checking anyway (type conversion for example), perhaps you could just throw a runtime error from app.route during startup. Mismatches aren't going to get far.

This has a few advantages:

• The user only needs to specify the type once, not twice.
• You'd need far less compile-time "black magic".
• It would be trivial to extend to user define types - any type constructible with a string.

Perhaps you could allow the user to define the placeholder (everyone has different preferences):

"/add/<_>/<_>"
"/add/{}/{}"
"/add/_1/_2"  // c.f. std::placeholders to allow reordering... ;-) 

from crow.

Some very interesting ideas here. However, this would depart from one of the core intentions of the current implementation, which is to compile-time validate that your handler function and URL parameters match types.

How would I differentiate...

app.route("/add/__/__", [](int a, int b) { ... });
app.route("/add/__/__", [](const char* a, int b) { ... });
app.route("/add/__/__", [](const char* a, const char* b) { ... });

(Not that this is necessarily good design, but at least the current system is prescriptive enough to cope with it.)

from crow.

You haven't lost anything, these routes could be handled in just the same way that crow currently handles them (i.e. just pass the argument list from the handler through to the Router::new_rule_tagged method, instead of looking up the same argument list using the black magic tag value).

The number of placeholders could easily be validated at compile-time. What type-safety do you gain by asking the user to specify the same parameter list twice?

from crow.

I really like this proposal. Also follows the DRY principle.

from crow.

I agree on the proposal. My purpose of describing the type in the url was make route look like Flask, and it is much better if we can remove macros. Also I want to find a way to add an argument with user defined type in handle function.

Crow is already checking types at the run time. It gives 400 Bad Request to the user if non-number is given to url argument(ex, request /add/abc/def to /add/<int>/<int>).

For user defined type conversion, I think it is good to add a template type like std::hash for it. One possible name is crow::to_argument, and if the argument type is int, crow::to_argument<int>(std::string) is called. Any user want to add a type conversion for a CustomType can specialize crow::to_argument<CustomType>::operator().

My preferred placeholder string is "%".

from crow.

My preferred placeholder would involve a numeral: %1%, {2}, <3>, _4

Like most mature string formatters, it seems reasonable to allow users to specify a route that doesn't necessarily specify arguments left-to-right.

from crow.

I added route_dynamic and can be used as follows:

    app.route_dynamic("/set_int/<int>")
    ([&](int y){
        x = y;
        return "";
    });

It checks handler type at run-time and throws exception if there's a mismatch.

from crow.

Awesome, nice one.

For reference, added in 5d8d527

from crow.

CROW_ROUTE is still kept; Compile-time checking is better to use I think.

from crow.

Isn't compile time checking a bit pointless here? There is the DRY principle issue and the browser user is free to put any input to the address bar, so we still need to check the handler arguments against the input at run-time and it should be enough to do it once instead of both compile-time and run-time.

from crow.

The point of compile-time checking is to help protect developers from themselves, not against users necessarily. Developers choose C++ because of compile-time safety (amongst other things) and so the point of this feature was to extend this safety to routes. I believe ipkn will admit that it was a bit of a personal goal, to leverage the new C++ features in order to achieve this however. If we get this 'for free' then I can't see the harm in it.

from crow.

Isn't compile time checking a bit pointless here?

Absolutely not. A huge draw to Crow is its compile-time checking. Following up on the previous suggestions, why not keep the route as a template parameter? Without the macro, we can have something along the lines of:

app.add<"/hello/<int>"_route>([](int count){ ... });

// or, without the UDL

app.add<crow::route("/hello/<int>")>([](int count){ ... });

The integer black magic for rebuilding the types is working well; we should keep it and just remove the icky macro.

from crow.

The main reason that I keep both CROW_ROUTE and route_dynamic is that
I can't find a way to implement the route function as @jeaye suggested.

If the route is only given to template part, the returned object of app.route or app.add can only see the parameter_tag (the integer value) and don't know the route itself. That's why I added url parameter and pass it to new_rule_tagged.

It seems that the only workaround is converting the string literal into variadic template of chars. But it needs much longer compile time, so I want to avoid it.

Please let me know if anyone can find a better way to do this.

from crow.

On Thu, Apr 16, 2015 at 04:06:56AM -0700, ipkn wrote:

It seems that the only workaround is converting the string literal into
variadic template of chars. But it needs much longer compile time, so I
want to avoid it.

This is a trade off I'd absolutely be willing to make, given that it's C++. Not everyone agrees though, no doubt.

from crow.