Jonathon,
Thank you very much for your input! All of the code that was in ApacheFopFactory is now in the
ApacheFopWorker.getFactoryInstance() method. It's not a big change and everything still works the same.
Getting singleton instances of classes from worker classes is done in other places in OFBiz
(FreeMarkerWorker.getDefaultOfbizConfig() for example), so I don't see a problem with doing the same
thing in this case.
-Adrian
Jonathon -- Improov wrote:
Adrian,
The ApacheFopFactory also handles some stuff like fopPath, fop config
file, base fonts defaults, etc.
Such attributes should be tied to the FopFactory. They certainly are not
attributes of a Worker class.
The point of having a FopFactory class distinct from a Worker class is
to cleanly encapsulate Factory-related attributes inside a single
entity. If we start putting Factory-related attributes into the Worker
class, we'd have a Worker that is overworked with a gigantic scope of
maybe these 2 things:
1. Manage setup and calls to the Factory.
2. Manage Factory attributes.
The singleton thing can be achieved with a static member "fopFactory"
inside the Worker. The FopFactory must be a singleton in OFBiz, true.
But that's not the point here about separating Factory and Worker. It's
about clean (and intuitive and logical) delegation of responsibilities
to Factory and Worker.
Jonathon
Adrian Crum wrote:
Jacopo Cappellato wrote:
Adrian,
I've not followed the details of your design, and so sorry if my
comment is out of scope here... the main reason for the
ApacheFopFactory class is that it implements a singleton.
Maybe you have already addressed this in another class but it was
worth of a mention.
Jacopo
Yes, that is addressed in the new class. To summarize: I created a
worker class that eliminates a lot of redundant FOP code, and that
worker class includes the singleton FopFactory instance.
-Adrian
