Fixed: FQDNs were incorrectly recognized as typos

Fixed: FQDNs were incorrectly recognized as typos

[Wladimir Palant, 2013-04-16 13:59:29]

LGTM with the suggested change.

http://codereview.adblockplus.org/10220011/diff/1/lib/typoFixer.js
File lib/typoFixer.js (right):

http://codereview.adblockplus.org/10220011/diff/1/lib/typoFixer.js#newcode134
lib/typoFixer.js:134: domain = domain.substr(0, domain.length - 1);
How about:

  domain = domain.replace(/\.$/, "");

[Thomas Greiner, 2013-04-16 14:06:39]

On 2013/04/16 13:59:29, Wladimir Palant wrote:
> LGTM with the suggested change.
> 
> http://codereview.adblockplus.org/10220011/diff/1/lib/typoFixer.js
> File lib/typoFixer.js (right):
> 
> http://codereview.adblockplus.org/10220011/diff/1/lib/typoFixer.js#newcode134
> lib/typoFixer.js:134: domain = domain.substr(0, domain.length - 1);
> How about:
> 
>   domain = domain.replace(/\.$/, "");

That was my first idea as well which is why I decided to compare the performance
between those two. Unfortunately, the regex was much slower (~4x) than the one I
ended up using. (tested 5,000,000 iterations each, once for a domain with and
once without trailing .)

[Wladimir Palant, 2013-04-17 07:47:41]

Sure, regexps applying to the end of a string are generally not very fast.
Still, unless you can show that this performance difference matters (in
particular, when compared with the performance of the matching) I would consider
it premature optimization - the performance gain isn't worth the decreased
readability.