-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat(isEAN): implement isEAN validator #1244
Conversation
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.
Thanks for this. LGTM! 🎉
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.
Looks great. Good job @hamzahejja! Can you take care of the 2 minor issues
src/lib/isEAN.js
Outdated
* with exact numberic matching of 8 or 13 digits [0-9] | ||
*/ | ||
const LENGTH_EAN_8 = 8; | ||
const validEanRegex = /(?<!\d)(\d{8}|\d{13})(?!\d)$/; |
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.
RegExp lookbehind assertions are an ES2018 feature. They are not supported in all platforms.
By the way, can't a much simpler regex like /^(\d{8}|\d{13})$/
do the trick?
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.
@tux-tn Yeah you're right, the /^(\d{8}|\d{13})$/
is sufficient to detect a consecutive sequence of 8-digits or 13-digits only ... Forgot about the lookbehind assertions being an ES2018 feature. Thanks! I've just modified it 👍
|
||
const remainder = 10 - (checksum % 10); | ||
|
||
return remainder < 10 ? remainder : 0; |
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.
The case where remainder equals 10 is uncovered in mocha tests. Can you add a test case?
@tux-tn Changes addressed accordingly 👍 |
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.
Thank you for your responsiveness Hamza. LGTM 🎉 !
EAN: European Article Number.
Wikipedia: International Article Number
isEAN(str)
string validator 🎉