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

Question about .valid? method #15

Open
jbutton opened this issue Feb 5, 2019 · 3 comments
Open

Question about .valid? method #15

jbutton opened this issue Feb 5, 2019 · 3 comments

Comments

@jbutton
Copy link
Contributor

jbutton commented Feb 5, 2019

Hello.
I think the ".valid?" method is not accurate.
as I can see it's only checking simple regexp isbn.match(/^[0-9]{13}$/) and checksum

It doesn't work well for many ISBNs, for example 8830114722800 - it is mark as valid but it is not:

https://isbnsearch.org/search?s=8830114722800

For now I can use workaround:
if lisbn.parts.nil? && lisbn.parts(4).nil?
then it is invalid.

I would like to look around for proper ISBN validation rules, and use it in ".valid?" method. I will do the research.

@ragalie
Copy link
Owner

ragalie commented Feb 9, 2019

I agree, I think the valid? method is poorly named right now. It really is more like valid_checksum?.

I'd be very happy to review a PR that uses the parts method to determine if the ISBN is really valid, and that renames the existing methods to valid_checksum?.

Thanks!

@slimdave
Copy link
Collaborator

slimdave commented Feb 12, 2019

Yes the real validity test would be both checksum and that the range has been issued by ISBN international. Making the methods #valid_isbn_13? and #valid_isbn_10?public might be helpful.

@jbutton
Copy link
Contributor Author

jbutton commented Feb 12, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants