Bug 41995

Created attachment 19849 [details]
AFP Barcode Patch

Created attachment 19850 [details]
New files associated to the patch

Pete, I took a look at your patch. I must say, I'm not happy with the approach
you've taken. You've copied many classes from Barcode4J, adjusted some of them
and put them in a new package under the FOP source tree. This is a maintenance
nightmare and the legal paperwork also needs to be considered.

That aside, I'm not comfortable with the bcoca package having a dependency into
the classes coming from Barcode4J. From an architecture perspective, it would be
much cleaner to simply provide the object model of the BCOCA functionality in
the bcoca package and then fill that from the Barcode extension. Like we do it
for formats like PDF, PostScript and RTF, I'd prefer to keep that pattern,
because that makes it possible to use the AFP code outside the FOP context, too.
Until now, that pattern was preserved. The package o.a.fop.render.afp.modca had
only dependencies into:
- o.a.fop.render.afp.exceptions
- o.a.fop.render.afp.fonts
- o.a.fop.render.afp.tools
- o.a.commons.logging

With your patch, o.a.fop.render.afp.bcoca and the forked Barcode4J classes are
added. Ideally, the modca package wouldn't even have a dependency into bcoca.
Right now, there's a circular dependency in your patch.

I have a proposal: Let's reuse (and adapt if necessary) Barcode4J and build a
cleaner dependency tree. Let's rid the bcoca package of references to Barcode4J
classes and only concentrate on the object model there. In a second step, let's
extend the FOP extension in Barcode4J to support native AFP barcodes. It's more
or less what I saw as the ideal solution in the first place but since you
introduced so many aspects from Barcode4J I think this is the right solution.
I can offer you some help with that. I've already locally removed all the forked
classes and started to correct the remaining issues. I would be comfortable with
just applying the part of the patch with the BCOCA object model after I've done
some changes. We can then go from there.

Please note that we once talked about bundling Barcode4J with FOP. We could take
this as an additional incentive to actually do it.

WDYT?

BTW, further advantages with my proposal:
- Less documentation overhead
- Same namespace as Barcode4J already uses (no special handling for AFP)
- Optimal rendering (native or Java2D) depending on what's already implemented.

Disadvantages:
- we cannot just use your patch, more development necessary
- we need to make sure that the FOP extension works with 0.93 until the next
release of FOP.

Jeremias, I should have made it clearer I never really expected the patch to be
applied as it stands for the reason that it relies on forked bc4j code. 

I submitted the patch in the hope that it would prompt this discussion, when
writing it I obviously had the issue of calculating the barcode dimensions which
is exactly what Barcode4J does very well, so didn't want to re-solve that problem.

My intention was not that that FOP would have a seperate set of barcode4j
classes as I agree it would be a mainatenance nightmare. This is why I put in
the barcode4j a seperate package, so that it could be reviewed as part of the
patch and you could decide the best way to achieve this with your Barcode4j hat on.

I'd like to see fop include barcode4j, but obviously it also stands on it's own
for users who want to generate images. I am more than happy with your proposal
and we can take it forward once you've finished the package refactor.

However I think some kind of bridging pattern would be required so that the
BCOCA attributes (to get the afp barcode type and modifier) can be obtained
based on the barcode being rendered.

(In reply to comment #4)
> BTW, further advantages with my proposal:
> - Less documentation overhead
> - Same namespace as Barcode4J already uses (no special handling for AFP)
> - Optimal rendering (native or Java2D) depending on what's already implemented.
> 
> Disadvantages:
> - we cannot just use your patch, more development necessary
> - we need to make sure that the FOP extension works with 0.93 until the next
> release of FOP.

Great! I feared we were too far apart on this, but apparently not. I'll see what
I can do today. It looks like AbstractBarcodeBean is tied more deeply into the
bcoca package than I first thought. How much time do you have to work on this? I
mean, if I can defer some of the work to you, I'd be more than happy. That would
allow me to concentrate on other FOP issues.

(In reply to comment #6)
I have limited time this week, but could progress further next week. However I'd
like to agree an approach, the AbstractBarcodeBean is basically tied deeply as
the bcoca model requires access to these attributes. 

I'd suggest that I make this an interface, and then the barcode4j ext can
provide a wrapper that implements that interface if using native support that
just delegates to the real bean. 

Rather than make this interface AFP specific this could go in a barcode package
so that any other renderers that are capable of native barcode support could use
the same mechanism.

This shouldn't take to long to do, WDYT?



> Great! I feared we were too far apart on this, but apparently not. I'll see what
> I can do today. It looks like AbstractBarcodeBean is tied more deeply into the
> bcoca package than I first thought. How much time do you have to work on this? I
> mean, if I can defer some of the work to you, I'd be more than happy. That would
> allow me to concentrate on other FOP issues.

I'm not sure if it's worthwhile creating such an interface. After all, it has to
map on all possible symbologies. On one side you have to map onto the
configurable beans and on the other side it has to map into the object model for
AFP. So the interface would be something like the superset of all barcode
implementation beans in Barcode4J.

I guess the one thing that needs to be done are accessors for all the different
values for the BCOCA object model. So rather than the BCOCA model requires
access to data, let's set the data on the object model.

BTW, you've created a class BarcodeDataDescriptor (BDA). Shouldn't that be
BarcodeSymbolDescriptor (BSD)? At least that's what it's called in my BCOCA
manual (S544-3766-06). OTOH, I still have to learn a few things about AFP. Maybe
I got it wrong.

Anyway, in the time I had left today I haven't been able to get to a
representable state. Maybe I'll do some more on Friday (depending on the weather).

The BDD specifies the size of the bar code presentation space, the type of bar
code to be generated, and the parameters used to generate the bar code symbols.

The data portion of the BDD structured field is defined in Bar Code Symbol
Descriptor (BSD), I didn't see much benefit in havng this in a seperate BSD
object. See page 112 of the BCOCA Reference manual.

I'm not sure that setters on the BCOCA model are a ideal solution. I think we'd
still need BCOCA objects representing the different barcodes and for these to
have the setters. This then starts to replicate the beans in barcode4j, hence
the interface suggestion.

For example the PDF417 barcode requires a special functions in the BarcodeData
object to set the error correction level and number of columns.

Let me know how you get on and if you want me to pick anything up.

(In reply to comment #9)
> Let me know how you get on and if you want me to pick anything up.

Earlier I said it depended on the weather. In the end, it was my computer that
made sure I take 4 days off: The power supply of the big box said goodbye after
5 minutes on Friday morning with a loud bang. I have to get a replacement part
first before I can access what little work I've started so far. And that'll take
a few days. It's probably best if you just continue the way you outlined it.

Created attachment 19929 [details]
Refactored afp barcode patch

This has been refactored to remove the dependency on barcode4j, it requires a
BarcodeSymbology object to be created, and therefore in order for barcode4j to
generate native BCOCA it will need to map to the appropriate object and set the
attributes accordingly. 

It might be worth including an afp extension that can generate BCOCA and perhap
allow the user to specify the presentation space. This would mean that it could
be used without calculating dimensions as per barcode4j.

The next step I guess is to get the barcode4j extension to support this native
interface.

Created attachment 19930 [details]
Zip file included with the new files

Created attachment 19947 [details]
Fix to change setters from int to double

Fix to change setters from int to double

resetting P2 open bugs to P3 pending further review

increase priority for bugs with a patch