atom / fuzzaldrin

Fuzzy filtering and string scoring
MIT License
319 stars 28 forks source link

Refactor spec so it fails consistently and properly retrieve filename. #14

Closed jmacdonald closed 10 years ago

jmacdonald commented 10 years ago

This corrects the previous deficient spec, and provides a proper implementation.

kevinsawicki commented 10 years ago

Mind rebasing this on master?

jmacdonald commented 10 years ago

Of course not. :wink:

kevinsawicki commented 10 years ago

My main concern here is speed, this current implementation to creates a couple arrays and also splits the entire string when only an end of string comparison is needed.

The benchmarks appear to suggest a 30-40% slower filtering time.

Current Master

> npm run benchmark

> fuzzaldrin@2.0.0 benchmark /Users/kevin/github/fuzzaldrin
> coffee benchmark/benchmark.coffee

Filtering 66672 entries for 'index' took 150ms for 6168 results
Matching 66672 entries for 'index' took 186ms for 6168 results

This PR

> npm run benchmark

> fuzzaldrin@2.0.0 benchmark /Users/kevin/github/fuzzaldrin
> coffee benchmark/benchmark.coffee

Filtering 66672 entries for 'index' took 217ms for 6168 results
Matching 66672 entries for 'index' took 179ms for 6168 results

Would you be cool tweaking it to not use split, but instead something like:

queryIsLastPathSegment = (string, query) ->
  index = string.lastIndexOf(query)
  index is string.length - query.length and string[index - 1] is '/'
jmacdonald commented 10 years ago

Good catch! Considering this is something that'll be run on a regular basis where performance is key, those gains are well worth it. :thumbsup:

kevinsawicki commented 10 years ago

Great, thanks for this.