Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in natural.JaroWinklerDistance() when missing optional options parameter #694

Closed
dandybytes opened this issue Aug 12, 2023 · 0 comments · Fixed by #699
Closed

Error in natural.JaroWinklerDistance() when missing optional options parameter #694

dandybytes opened this issue Aug 12, 2023 · 0 comments · Fixed by #699

Comments

@dandybytes
Copy link

The example in the following documentation link seems to imply that the natural.JaroWinklerDistance() function can be used with two required arguments (i.e. the two strings to compare) and an optional options parameter:

https://naturalnode.github.io/natural/string_distance.html

However, according to the function I can see in code, the options parameter is required. If you do not pass it, then the function throws an error, because options === undefined and options.ignoreCase is undefined.ignoreCase, which obviously fails.

https://github.com/NaturalNode/natural/blob/c2389f2a17faae9582a1a0f41402c68407ccb378/lib/natural/distance/jaro-winkler_distance.js#L109C1-L109C30

A) If the options argument is indeed optional, then I would use the optional chaining operator in all applicable cases (i.e. options.ignoreCase ---> options?.ignoreCase and options.dj ---> options?.dj)
B) If the options parameter is meant to be required now, then I would update the documentation and the function comments accordingly, and maybe add a condition to that function that checks for the presence of the options and, perhaps, use the optional chaining operator for extra safety anyway.

By the way, those function comments would greatly benefit from proper JSDoc annotations, in addition to clearer explanation of the data structure of that options parameter.

Otherwise, thanks for the great job, guys!

MukeshSinghBisht pushed a commit to MukeshSinghBisht/natural that referenced this issue Sep 8, 2023
Hugo-ter-Doest pushed a commit that referenced this issue Sep 11, 2023
…ions parameter #694 (#699)

* aggressive tokenization hindi added

* Update aggressive_tokenizer_hi_spec.js

corrected test title

* Copyright text update as per requested changes

* fixed #694

* reverted unwanted for this specific commit

* lint error fixed

---------

Co-authored-by: Mukesh Singh Bisht <mukesh.bisht@brainvire.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant