UrlRouter which uses regular expressions to find out what to do

Question

I have created UrlRouter. It should call appropriate controllers methods with extracted values as a parameters. It works fine, but my boss told me that I should make it better. Could you give me some advice on how to improve this class, please?

public class UrlRouter {

    private final static String pageRegex = "/page/\\d{1,5}";
    private final static String yearRegex = "/\\d{4}";
    private final static String monthRegex = "/\\d{1,2}";
    private final static String nameRegex = "/[a-z0-9\\-]+";

    private final static Pattern page = Pattern.compile(pageRegex + "$");
    private final static Pattern year = Pattern.compile(yearRegex + "$");
    private final static Pattern yearPage = Pattern.compile(yearRegex + pageRegex + "$");
    private final static Pattern yearMonth = Pattern.compile(yearRegex + monthRegex  + "$");
    private final static Pattern yearMonthPage = Pattern.compile(yearRegex + monthRegex + pageRegex  + "$");
    private final static Pattern yearMonthName = Pattern.compile(yearRegex + monthRegex + nameRegex  + "$");
    private PostController postController;

    public UrlRouter(PostController controller) {
        this.postController = controller;
    }

    public void route(HttpServletRequest request) {
        String url = request.getRequestURI();

        if (yearMonthName.matcher(url).find()) {
            showOne(url);
        } else if (yearMonthPage.matcher(url).find()) {
            showMonthList(url);
        } else if (yearMonth.matcher(url).find()) {
            showMonth(url);
        } else if (yearPage.matcher(url).find()) {
            showYearList(url);
        } else if (year.matcher(url).find()) {
            showYear(url);
        } else if (page.matcher(url).find()) {
            showPage(url);
        } else {
            postController.showPage(1);
        }
    }

    private void showPage(String url) {
        String[] parts = url.split("/");
        int pageNumber = Integer.parseInt(parts[parts.length-1]);
        postController.showPage(pageNumber);
    }

    private void showYear(String url) {
        String[] parts = url.split("/");
        int year = Integer.parseInt(parts[parts.length - 1]);
        postController.showYear(year, 1);
    }

    private void showYearList(String url) {
        String[] parts = url.split("/");
        int year = Integer.parseInt(parts[parts.length - 3]);
        int page = Integer.parseInt(parts[parts.length - 1]);
        postController.showYear(year, page);
    }

    private void showMonth(String url) {
        String[] parts = url.split("/");
        int month = Integer.parseInt(parts[parts.length - 1]);
        int year = Integer.parseInt(parts[parts.length-2]);
        postController.showMonth(year, month, 1);
    }

    private void showMonthList(String url) {
        String[] parts = url.split("/");
        int year = Integer.parseInt(parts[parts.length - 4]);
        int month = Integer.parseInt(parts[parts.length - 3]);
        int page = Integer.parseInt(parts[parts.length - 1]);
        postController.showMonth(year, month, page);
    }

    private void showOne(String url) {
        String[] parts = url.split("/");
        int year = Integer.parseInt(parts[parts.length - 3]);
        int month = Integer.parseInt(parts[parts.length - 2]);
        String name = parts[parts.length - 1];
        postController.showOne(year, month, name);
    }
}

Vogel612 · Accepted Answer · 2015-11-10 21:38:19Z

Unless you are deliberately studying regular expressions, you should look into Jersey - UriTemplate.match() would probably make your life a lot easier.

public void route(HttpServletRequest request) {
    String url = request.getRequestURI();
    // ...
}

First thing to notice here, is that you don't use the request object after you have extracted the uri from it. Therefore, the work should be delegated to another method that doesn't have the HttpServletRequest dependency

public void route(HttpServletRequest request) {
    route(request.getRequestURI());
}

public void route(String uri) {
    // ...
}

Now, direct your attention to your if clauses

    if (yearMonthName.matcher(url).find()) {
        showOne(url);
    } else if (yearMonthPage.matcher(url).find()) {
        showMonthList(url);
    } else if (yearMonth.matcher(url).find()) {
        showMonth(url);
    } else if (yearPage.matcher(url).find()) {
        showYearList(url);
    } else if (year.matcher(url).find()) {
        showYear(url);
    } else if (page.matcher(url).find()) {
        showPage(url);
    } else {
        postController.showPage(1);
    }

In the large, what this code does is switch behaviors based on a match; changing behaviors should be modeled with objects. If you squint a bit, you should be able to see that what you are really doing here is

Using the uri to look up an action
Executing the action

Therefore, you should make those ideas explicit.

interface RequestHandler {
    void handle(String uri, PostController postController);
}

public void route(String uri) {
    RequestHandler handler = lookup(uri);
    handler.handle(uri, this.postController);
}

RequestHandler lookup(final String uri) {
    if (yearMonthName.matcher(url).find()) {
        return new RequestHandler() {
            public void handle(String uri, PostController postController){
                String[] parts = url.split("/");
                int year = Integer.parseInt(parts[parts.length - 3]);
                int month = Integer.parseInt(parts[parts.length - 2]);
                String name = parts[parts.length - 1];
                postController.showOne(year, month, name);
            }
        }
    }
    // ...
}

Of course, there's no need for us to crate a new RequestHandler every time -- it doesn't have any state.

private static final RequestHandler SHOW_ONE_HANDLER = new RequestHandler() {
            public void handle(String uri, PostController postController){
                String[] parts = url.split("/");
                int year = Integer.parseInt(parts[parts.length - 3]);
                int month = Integer.parseInt(parts[parts.length - 2]);
                String name = parts[parts.length - 1];
                postController.showOne(year, month, name);
            }
         };

RequestHandler lookup(final String uri) {
    if (yearMonthName.matcher(url).find()) {
        return SHOW_ONE_HANDLER;
    }
    // ...
}

The big advantage of this approach is that we can simplify lookup quite a bit

private static final Map<Pattern,RequestHandler> LOOKUP_TABLE = new HashMap();

static {
    LOOKUP_TABLE.put(yearMonthName, SHOW_ONE_HANDLER);
}

RequestHandler lookup(final String uri) {
    RequestHandler currentHandler = DEFAULT_HANDLER;
    for(Map.Entry<Pattern, RequestHandler> entry : LOOKUP_TABLE.entrySet()) {
        Pattern pattern = entry.getKey();
        if (pattern.matcher(uri).find()) {
            currentHandler = entry.getValue();
            break;
        }
    }
    return currentHandler;
}

Now, there is an additional improvement to consider -- it's kind of silly that we parse the url to figure out the right event handler, then parse the url again within the handler. You do a little bit better using the factory pattern.

interface RequestHandler {
    void handle(PostController postController);
}

interface RequestHandlerFactory {
    RequestHandler create(String uri);
}

    private static final RequestHandlerFactory SHOW_ONE_FACTORY = new RequestHandlerFactory() {

    public RequestHandler create(final String uri) {
        return new RequestHandler() {
            public void handle(PostController postController) {
                String[] parts = uri.split("/");
                int year = Integer.parseInt(parts[parts.length - 3]);
                int month = Integer.parseInt(parts[parts.length - 2]);
                String name = parts[parts.length - 1];
                postController.showOne(year, month, name);
            }
        };
    }
};

//...
RequestHandler lookup(final String uri) {
    for(Map.Entry<Pattern, RequestHandlerFactory> entry : LOOKUP_TABLE.entrySet()) {
        Pattern pattern = entry.getKey();
        if (pattern.matcher(uri).find()) {
            return entry.getValue().create(uri);
        }
    }
    return DEFAULT_HANDLER;
}

The advantage to this pattern, is that it works well if decide to extract your parameters some other way. For instance, if you start using groups in your regular expressions, then you could....

interface RequestHandlerFactory<T> {
    RequestHandler create(T uri);
}

RequestHandler lookup(final String uri) {
    for(Map.Entry<Pattern, RequestHandlerFactory<Matcher>> entry : LOOKUP_TABLE.entrySet()) {
        Pattern pattern = entry.getKey();
        final Matcher m = pattern.matcher(uri);
        if (m.find()) {
            return entry.getValue().create(m);
        }
    }
    return DEFAULT_HANDLER;
}

or if you were to use UriTemplates instead of rolling your own regular expressions

RequestHandler lookup(final String uri) {
    Map<String,String> uriParams = new HashMap<String, String>();

    for(Map.Entry<UriTemplate, RequestHandlerFactory<Map<String,String>>> entry : LOOKUP_TABLE.entrySet()) {
        UriTemplate pattern = entry.getKey();
        if (pattern.match(uri, uriParams)) {
            return entry.getValue().create(uriParams);    
        }
    }
    return DEFAULT_HANDLER;
}

Vogel612 · Accepted Answer · 2015-11-08 15:44:40Z

The first improvement you definitely want is capturing groups. These groups allow you to get rid of splitting at "/" and working on magic indices and instead use a Matcher to extract the parts from a matched string.

This greatly simplifies your delegating methods that work on a String to using the Matcher instance you used and threw away in your if-condition:

private void showPage(Matcher url) {
    postController.showPage(url.group(1));
    // alternatively use
    postController.showPage(Integer.parseInt(url.group(1)));
}

This would only require minor changes for your regexes:
You'd need to enclose the captured groups in parentheses as follows.

private final static String pageRegex = "/page/(\\d{1,5})";
private final static String yearRegex = "/(\\d{4})";
private final static String monthRegex = "/(\\d{1,2})";
private final static String nameRegex = "/([a-z0-9\\-]+)";

Thank you for this suggestion. I will learn how to use capturing groups. — Iaroslav Karandashev, Commented Nov 8, 2015 at 23:12

janos · Accepted Answer · 2015-11-09 04:48:11Z

2

A few minor points on top of Vogel's:

It seems PostController postController can be final
Instead of using request.getRequestURI() (the entire URL) and Matcher.find(), you could use request.getContextPath() (just the path part) and Matcher.matches(). The pattern matching logic would be slightly simpler that way, and you could drop the $ from the end of the patterns

edited Nov 9, 2015 at 4:48

answered Nov 8, 2015 at 16:39

janos

112k15 gold badges152 silver badges391 bronze badges

Add a comment |

Stack Exchange Network

UrlRouter which uses regular expressions to find out what to do

3 Answers 3

Your Answer

Not the answer you're looking for? Browse other questions tagged
java
regex
url-routing
servlets
or ask your own question.

Hot Network Questions

UrlRouter which uses regular expressions to find out what to do

3 Answers 3

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged javaregexurl-routingservlets or ask your own question.

Related

Hot Network Questions

Not the answer you're looking for? Browse other questions tagged
java
regex
url-routing
servlets
or ask your own question.