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

Invalid GTIN prevent getting other useful information #157

Closed
mfaheemkhan6 opened this issue Oct 23, 2022 · 4 comments · Fixed by #169
Closed

Invalid GTIN prevent getting other useful information #157

mfaheemkhan6 opened this issue Oct 23, 2022 · 4 comments · Fixed by #169
Labels
breaking Backwards incompatible change
Milestone

Comments

@mfaheemkhan6
Copy link

It is actually a feature request.
Sometimes in internal usage, we don't care for a valid GTIN. In parsers, there should be an option to skip check for invalid GTIN.
For example. Something like:
biip.gs1.GS1Message.parser(code, fnc1=some_fnc1, check_gtin=False)

If AI 01 or 02 is 14 digit long, it is okay. No need to check for check digit. It will save a little time and system resources if an organization has all valid GTIN, and they don't want to check its validity, and it will also enable an organization to parse any 14-digit code (valid or invalid) for internal use.
Furthermore, currently, if someone wants to get only Lot number or serial number or any other information other than GTIN, if the code has invalid GTIN, he is unable to get any other information because GTIN validation is returning the function with exception.
So, if someone wants to skip check for Valid GTIN, he should have a bool argument to send to the parser function.

@jodal
Copy link
Owner

jodal commented Oct 23, 2022

I see your point. I might consider changing the GS1 parser so that it just sets the GS1ElementString.gtin field to None if validating the GTIN fails. That way you get a GS1Message object in return and can access e.g. the lot number AI. You'd be able to get the raw GTIN string from GS1ElementString.value.

Does that sound like a solution that would work for you?

@mfaheemkhan6
Copy link
Author

Thank you very much for taking it into consideration. Actually, my opinion is to make it optional but set default to True, so people already using it may not affect. I explained the reason, if in an organization they have 100000 gs1 codes, and they need to scan them daily 500K times, and they already know that all of them are valid, why do they check it again and again on each scan? Skipping this check will save time and resources, even if the difference is minor.

I did fork this repo and updated it a little. It is now working as I wanted. Maybe my approach is not correct, but it is working in my current implementation. I did not check if other parts are working or not, but gs1 parser is working. If I send check_gtin=False, it is returning the 14-digit code as GTIN but for check digit it is returning "8: invalid, should be: 7" so we can know that the GTIN was not valid but still we can use this 14-digit code.

Anyway, 1st part of your proposed solution is perfect, this way at least other parts of barcode are accessible, and for GTIN, if you are not willing to skip the check, then it can be provided with some other name, like you did for date field, you can extract raw date or datetime.date from the message, similarly you can provide it as invalid_GTIN or any similar name. Anyway, this is a great work. I really appreciate your work. I hope that you will update it soon.

@jodal jodal added this to the 3.0 milestone Nov 30, 2022
@jodal jodal added bug Something isn't working breaking Backwards incompatible change labels Nov 30, 2022
@jodal
Copy link
Owner

jodal commented Dec 18, 2022

I explained the reason, if in an organization they have 100000 gs1 codes, and they need to scan them daily 500K times, and they already know that all of them are valid, why do they check it again and again on each scan? Skipping this check will save time and resources, even if the difference is minor.

The extra resources required by validating 500k barcodes using Biip, which involves no extra I/O, only a bit of CPU, is entirely minimal, so I don't give this argument too much weight.

My experience from scanning also something like 500k barcodes daily in the fulfillment centers of an online grocery store is that there is astonishing amounts of broken barcodes in circulation, and validation is important to stop the problem close to the source, so that suppliers can be notified that their barcodes are invalid instead of accumulating invalid data.

@jodal
Copy link
Owner

jodal commented Dec 19, 2022

Biip 3.0.0 has now been released. It includes the changes discussed here, hopefully making Biip useful in your use case.

@jodal jodal removed the bug Something isn't working label Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Backwards incompatible change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants