Barcode format rework #2797
Barcode format rework #2797
Conversation
There was a problem hiding this comment.
For price barcode parsing... as set by the barcode format -> "02{P:2}{W:6}{I:3}"
Usually in digital scales, the price comes at the end of the barcode replacing W: so the final format will be something like 02{ID:5}{P:6} or as it was set before 02(\d{5})(\w{6}).
Setting the price for 2 digits might only work with USD currency so also might vary upto 5 or 6 digits.
|
I started to test it, but when I entered the format @WebShells gave as an example ("02{P:2}{W:6}{I:3}") into the Input Format ended up being rendered as follows. Is this correct? |
|
@SteveIreland I guess jekkos changed the format after the rework to "02{P:2}{W:6}{I:3}" |
| @@ -78,6 +78,24 @@ public function scan($text) | |||
| return $token_tree; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
This is more a philosophical question than a statement. What are our best practices for function comments? I've been trying to generally use this (https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#1-functions-class-methods) but I noticed that it isn't consistent throughout the code. Should we be asking everyone to comment their non-obvious functions in PR's?
There was a problem hiding this comment.
Well I would say better to have no comments at all (except if you really have no other choice). Code should be self explanatory where possible. Most of the time it's considered a code smell.
But for most php formatting I think we try to follow the CI guidelines.
| @@ -78,6 +78,24 @@ public function scan($text) | |||
| return $token_tree; | |||
| } | |||
|
|
|||
| public function parse_barcode(&$quantity, &$price, &$item_id_or_number_or_item_kit_or_receipt) | |||
There was a problem hiding this comment.
Is the reason for passing these by reference instead of a return value because there are multiple values being modified and you would have to put them into an array or object if you used return?
There was a problem hiding this comment.
Yes indeed that's the reason.
| @@ -0,0 +1,73 @@ | |||
| <?php | |||
There was a problem hiding this comment.
Sorry, another philosophical question. Do we want to begin asking the PR submitter to have Unit Tests written when new methods are being submitted?
There was a problem hiding this comment.
I would hope it would only be for areas that are delicate and complicated by configuration options so that we can have confidence that when someone makes a change in those areas that the appropriate unit tests can be run to cover all possible configuration permutations. I don't see a need to make simple changes so burdensome that we need to have unit tests for stuff that only have a single test scenario.
There was a problem hiding this comment.
I think that adding unit tests really adds a lot of value, we can only do it step by step from now on and it really helps to detect regressions early on, especially in more complex and critical sections of the code (eg calculations).
I think we should really try to add more unit tests. It's just a more effective way of writing code (test driven development) and validating features in the code. Probably not everything is testable in this project, like logic in the views is probably out of scope. But queries in the datamodel, functions in libraries and helpers all seem to be good and easy testable components.
Not sure if it works as expected, I'll have to review . |
This rework allows us to use barcode formats with the token system originally developed by @SteveIreland . Also an extra $price variable was added that can be used with a barcode scanner to fill in item parameters in sales and receivings directly.
Also added a couple of units tests to show how the functionality is working.