Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
Additional: vinUS validation fails on valid vin numbers#2460
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Compare by value and type (===) does not work for this algorithm, as both cd and cdv can be either types at the same time.By comparing by value only (==) cd and cdv can be either integer or string, as a string number will be converted to a number reqardless of type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please could you add a test, and revert the other changes as line 50 (if ( parseInt(cd) === parseInt(cdv) ) {) seems to be the only relevant change.
src/additional/vinUS.js Outdated
| } | ||
| if ( cd === cdv ) { | ||
| // cd and cdv can be either integer string | ||
| if ( cd == cdv ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I thinkparseInt() should be used instead. For example,(parseInt("X") === parseInt("X")) === false. Any complaint against that?
| if(cd==cdv){ | |
| if(parseInt(cd)===parseInt(cdv)){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
No, that will not work, if string cd ="X" and string cdv="X" should = true.
line 50 could be 'if (+cd === +cdv')' the plus(+) on a string will evaluate it to an int, other wise it is a string.
In reality, the vin check digit in location 8 should be transliterated to a number not a string.
That way the check digit cd and check digit vin cdv will always be a number.
I've rewritten the for loop, and removed the nested loop, different commit on my branch. |
Test cases include default test with 17 one's, and additional US and Canada VIN
wewhite commentedDec 1, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Code is better now, replaced nested for loop with an array index lookup. |
Thanks ❤️ |
Compare by value and type (===) does not work for this algorithm, as both cd and cdv can be either types at the same time. By comparing by value only (==) cd and cdv can be either integer or string, as a string number will be converted to a number reqardless of type.
Relates to BUG:#2459