PDF417: Automated structured append

[uwolfer]

Resolves #92

[gredler]

Thanks, will review this week!

[uwolfer]

@gredler I have rebased this change and applied slight improvements. Please let me know what you think.

[gredler]

Will do -- this is high on my to-do list.

[uwolfer]

FYI, I did now also extensive manual testing. Generated a series of e-tax 27 barcodes with both J4L and Okapi with the same input, and compared the output. All generated barcodes are 100% equal (pixel-by-pixel).

[gredler]

I'm a little hesitant to coordinate this code review via PR comments, but let's give it a try and see how it goes :-)

One high-level item to start: Can you resolve the conflicts? It looks like I caused a one- or two-line conflict with my recent line ending fix.

[uwolfer]

Can you resolve the conflicts?

Done.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 98.61111% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 87.31%. Comparing base (1a8bfa2) to head (31016ab).

Files	Patch %	Lines
...apibarcode/backend/Pdf417AutoStructuredAppend.java	98.61%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##             master     #103      +/-   ##
============================================
+ Coverage     87.25%   87.31%   +0.05%     
- Complexity     4299     4317      +18     
============================================
  Files            68       69       +1     
  Lines         13934    14006      +72     
  Branches       3254     3264      +10     
============================================
+ Hits          12158    12229      +71     
- Misses         1126     1127       +1     
  Partials        650      650              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[gredler]

Reviewed original PR last week

[uwolfer]

@gredler I have now added more test cases which cover boundary cases, and thus also found & fixes some issues uncovered by these.

[gredler]

I think the PR is looking good! I need to dig into the core of splitData a little more, but I think we're close. In the meantime, can you make a couple of small tweaks?

Let's move the implementation from Pdf417AutoStructuredAppend to Pdf417 itself, at the bottom of the class. I know it's already a long class, but that's the Zint / C code legacy, and we have to tread a fine line between "clean" Java code and not deviating too much from the original code (to make issues easier to diagnose and resolve). The approach (consistent with the original C code) is to have one file / class per symbol, and maintaining that consistency is probably more valuable than making things "cleaner" in one small corner of the code.

The JavaDoc for the byte[] version mentions setContent(byte[]), which is not used. It probably doesn't need to get too specific about how it works under the covers.

I think we can rename plotStructuredAppendSymbols to createStructuredAppendSymbols for consistency with the other methods.

The copyright in the Pdf417AutoStructuredAppendTest file can be changed to your name.

I'll try to check that last little bit in splitData later today!

[uwolfer]

Thanks. Just pushed an update which addresses your comments.

[uwolfer]

@gredler Do you think we can limit the high? I.e. is there a way to calculate the maximum ever possible data amount for a specified symbol? That would allow us to optimize performance.

(Adding comment again there in order to make it visible after moving code around.)