Skip to content

Commit

Permalink
Significantly improved isValid performance.
Browse files Browse the repository at this point in the history
Older implementation relied on iterating over each character of id(M)
and every time searching for that character in alphabet(N).
O(M * N), which seemed not too fast.

Rewritten it to use regular expressions, which boosted performance.

Here is the benchmark - https://jsperf.com/regex-vs-indexof-in-loop-2312
Used 1000 different generated ids, half of them were incorrect.

On average, the current algorithm would score ~1.3k op/sec
On the other hand, the new one makes around ~12.5k op/sec

Which I've found significant enough to make a PR.
  • Loading branch information
snqb authored and ai committed Jul 8, 2018
1 parent 7eea65a commit 04fc220
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 8 deletions.
10 changes: 2 additions & 8 deletions lib/is-valid.js
Expand Up @@ -6,14 +6,8 @@ function isShortId(id) {
return false;
}

var characters = alphabet.characters();
var len = id.length;
for(var i = 0; i < len;i++) {
if (characters.indexOf(id[i]) === -1) {
return false;
}
}
return true;
var nonAlphabetic = new RegExp('[^' + alphabet.characters() + ']+', 'g');
return !nonAlphabetic.test(id);
}

module.exports = isShortId;
1 change: 1 addition & 0 deletions test/is-valid.test.js
Expand Up @@ -16,6 +16,7 @@ describe('testing is-valid', function() {

it('should find these invalid', function(done) {
expect(isValid('i have spaces')).to.equal(false);
expect(isValid('i have \n breaks \n of \n the \n lines')).to.equal(false);
expect(isValid('abc')).to.equal(false);
expect(isValid(1234)).to.equal(false);
expect(isValid()).to.equal(false);
Expand Down

0 comments on commit 04fc220

Please sign in to comment.