deleteman / vatican

Micro-framework designed to create REST APIs with minor effort
98 stars 6 forks source link

findMethod method from vatican.js #12

Closed smart--petea closed 10 years ago

smart--petea commented 10 years ago

There are the method

Vatican.prototype.findMethod = function( url, method ) {
    var path = (url.indexOf("?") == -1) ? url : url.split("?")[0];
    var nmbrOfParts = path.split("/").length;

    var match = _.find(this.paths, function(item) { 
        var regExpStr = item.url.replace(/:.+(\/)?/g,".+$1");
        var nmbrOfPartsPath = item.url.split("/").length;

        var regExp = new RegExp(regExpStr + "$");
        return regExp.test(path) && (item.method.toLowerCase() === method.toLowerCase()) && nmbrOfPartsPath == nmbrOfParts;
    });
    return match;
};
  1. why are nmbrOfPartsPath and regExp computed on every request at every cycle of _.find. Maybe to replace it to handlerParser.js in parse function?
  2. by default in node.js request.method is uppercase. If in parse function (from handlerParser.js) the method will be set to uppercase then the expression
item.method.toLowerCase() === method.toLowerCase()

become

item.method === method
  1. colon symbol can be used not only for parameter but and as a symbol of url. This case can be catched by tranforming regexp
 var regExpStr = item.url.replace(/:.+(\/)?/g,".+$1");

to

 var regExpStr = item.url.replace(/\/:.+(\/)?/g,".+$1");
  1. change the order of comparation in (by complexity of operation)
        return regExp.test(path) && (item.method.toLowerCase() === method.toLowerCase()) && nmbrOfPartsPath == nmbrOfParts;

to

    return nmbrOfPartsPath == nmbrOfParts && (item.method.toLowerCase() === method.toLowerCase()) && regExp.test(path);

After the apply of these points the function will be

Vatican.prototype.findMethod = function( url, method ) {
    var path = (url.indexOf("?") == -1) ? url : url.split("?")[0];
    var nmbrOfParts = path.split("/").length;

    var match = _.find(this.paths, function(item) { 
         return   nmbrOfPartsPath == nmbrOfParts && (item.method === method)  && item.regExp.test(path);
    }) ;
    return match;
};
smart--petea commented 10 years ago

For now vatican.findMethod is

Vatican.prototype.findMethod = function( url, method ) {
.    ..
    var match = _.find(this.paths, function(item) { 
    ...
        return  nmbrOfPartsPath == nmbrOfParts && (item.method.toLowerCase() === method.toLowerCase()) && regExp.test(path);
    });
    return match;
}

In handlerParser.parse method is uppercased. It can be optimized by

  1. remove the calls for toLowerCase function
  2. uppercase method (parameter of function) at the begin of function

Function become

Vatican.prototype.findMethod = function( url, method ) {
.    ..
    method = String(method).toUpperCase();
    var match = _.find(this.paths, function(item) { 
    ...
        return  nmbrOfPartsPath == nmbrOfParts && (item.method === method) && regExp.test(path);
    });
    return match;
}
smart--petea commented 10 years ago

In findMethod the variables regExpStr and nmbrOfPartsPath are computed at every cycle. It is expensive, isn't it? Maybe to move these computations in handlerParser.parse method.