Fix for bug #593 #594
Merged
Fix for bug #593 #594
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Bug #593 was misleadingly reported as pertaining to another project, but testing revealed it affected this project as well. The Code128 encoding algorithm failed when an odd number of digits were followed by FNC1 and other digits. The code selection algorithm was rewritten. A test case showing the bug was added, and an old test case was updated to expect the result of the new algorithm - another valid encoding of the same length as the old one.
Bug #593 was misleadingly reported as pertaining to another project, but testing revealed it affected this project as well. The Code128 encoding algorithm failed when an odd number of digits were followed by FNC1 and other digits. The code selection algorithm was rewritten. A test case showing the bug was added, and an old test case was updated to expect the result of the new algorithm - another valid encoding of the same length as the old one.
Removed imports I didn't use after all.
| int end = start + length; | ||
| // Returns 1 for FNC_1, 2 for a pair of digits, -1 for a single digit, | ||
| // 0 otherwise | ||
| private static int codeCLength(CharSequence value, int start) { |
There was a problem hiding this comment.
Is it clearer just to make a private enum to express the possible return types?
Added a private enum and changed some names to improve readability. Also some trivial cosmetic changes.
|
Thanks for comments, did the changes. |
| @@ -90,6 +108,10 @@ public void testEncodeWithFunc4() throws WriterException { | |||
| assertEquals(expected, actual); | |||
| } | |||
|
|
|||
| private static BitArray matrixToArray(BitMatrix result) { | |||
There was a problem hiding this comment.
This is just used one place, and it doesn't really make something into an array, just returns one row right?
|
OK minus two more tiny comments, this seems fine |
Removed a private one-liner method used exactly once, alphabetized imports.
|
Done. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I fixed bug #593 and added one test case. My commits look a bit funny as I forgot the commandment, "Thou shalt not amend an already published commit."
I know the licensing policy and agree to it.