Seems a bit counter-intuitive, that design pattern. Or we could rename ApacheFopWorker to
ApacheFopFactoryHelper.
In any case, as long as we're decoupling correctly, the code organization will be crisp and clear,
easy to extend in future. We can deal with the names and delegation later, if we want to.
Jonathon
Adrian Crum wrote:
Jonathon and Jacques,
Thanks for the input!
Actually, I ended up going the other way around -
ApacheFopFactory.instance() does nothing more than call
ApacheFopWorker.getFactoryInstance().
I'll have a patch submitted to Jira in a few days for everyone to review.
-Adrian
Jacques Le Roux wrote:
Yes, good idea. I agree with Jonathon in keeping ApacheFopFactory or
paraphrasing Adrian "create a new class that references
ApacheFopFactory"
Jacques
De : "Jonathon -- Improov" <[EMAIL PROTECTED]>
Nice!
I would recommend keeping ApacheFopFactory, and then creating
ApacheFopWorker to use
ApacheFopFactory. Keep the 2 de-coupled; we never know when we gotta
extend ApacheFopFactory.
> If the FOP API changes, only the ApacheFopWorker would have to be
updated.
Oh, you're so right. I did some work for an Opentaps client who
blindly updated his Opentaps
installation with both Opentaps and OFBiz recent updates. Yup, wrong
medication and complications
all over the place (hence my recent post with "OFBiz updates as
medication" metaphor).
One sticky issue was the old FOP in his Opentaps. His PDFs weren't
showing up correctly (or was it
just barcodes?). I had a difficult time upgrading his codes to
support a newer FOP.
Your ApacheFopWorker will save us a lot of trouble as OFBiz grows
together with FOP.
Jonathon
Adrian Crum wrote:
Currently, the ApacheFopFactory class does nothing more than return a
FopFactory class instance. Looking through the project, I see a lot of
redundant code to render documents via FOP. I'd like to refactor the
ApacheFopFactory to include some helper methods that would eliminate
much of the redundant code.
Here is what I have in mind:
1. Rename ApacheFopFactory to something like ApacheFopWorker, OR create
a new class that references ApacheFopFactory.
2. Move a lot of the stream preparation and rendering code to
ApacheFopWorker.
3. Update existing OFBiz code to use the new class.
There are two things I can see being accomplished:
1. Code reduction.
2. Decouple FOP code from the multitude of classes that currently use
it. If OFBiz code only called ApacheFopWorker class methods, then the
code base would be shielded from FOP API changes. If the FOP API
changes, only the ApacheFopWorker would have to be updated.
It would be the same basic concept as the current FreeMarkerWorker
class.
What do you think?
-Adrian
