Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added IHttpRouter interface #177

Merged
merged 3 commits into from
Feb 7, 2013
Merged

Added IHttpRouter interface #177

merged 3 commits into from
Feb 7, 2013

Conversation

lclarkmichalek
Copy link
Contributor

I've been implementing my own router class recently, and I've felt the need for an interface for Http Routers in two ways:

  • The UrlRouter public API is 90% convenience functions, with only 2 functions that actually do work (match and handleRequest). By using an interface, the convenience functions need not be explicitly implemented by any other routers, and new routers need only implement match and handleRequest.
  • Having a base type for routers is really useful for creating generic/pluggable site components. I don't know what the standard way of organising vibe.d modules is, but I generally just expose 1 function to register routes. Currently it's not really possible to do this without specifying the type of the router, which isn't really great. Having a base type solves this problem.

This pull request adds a IHttpRouter interface that implements all of the convenience functions from UrlRouter and also modifies UrlRouter slightly to implement IHttpRouter (I changed the return value of match from void to IHttpRouter).

@s-ludwig
Copy link
Member

s-ludwig commented Feb 6, 2013

Just out of curiosity, what does your router do different/additional?

In general I don't see any issue adding an interface, I'll just make some small inline notes that would be good to have handled before merging.

@@ -16,6 +16,68 @@ import std.functional;


/++
An interface for HTTP request routers.
+/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: would be good to have /** .. */ style comments for consistency

@lclarkmichalek
Copy link
Contributor Author

I have 2 different routers. One which uses regex based parsing with named subgroups for parameter selection a la django (this relies on a patch to phobos that has yet to be merged) and another that allows the exposing of another router on a subpath i.e.

auto siterouter = new UrlRouter();
siterouter.get("/test", &testView);

auto forwardedrouter = new ForwardedRouter();
forwardedrouter.add_router("/subsite", siterouter);
forwardedrouter.get("/test", &testView2)
// So forwardedrouter will map GET /subsite/test -> testView, GET /test -> testView2

@s-ludwig
Copy link
Member

s-ludwig commented Feb 7, 2013

Thanks, merging in...

s-ludwig added a commit that referenced this pull request Feb 7, 2013
@s-ludwig s-ludwig merged commit 544f569 into vibe-d:master Feb 7, 2013
s-ludwig added a commit that referenced this pull request Feb 7, 2013
…ly with the latest style practice.

See also pull #177.
@lclarkmichalek lclarkmichalek deleted the IHttpRouter branch February 7, 2013 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants