Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java

2004-10-15 Thread Jeremias Maerki
Sorry for the delay.

On 12.10.2004 01:20:54 Glen Mazza wrote:
 --- Jeremias Maerki [EMAIL PROTECTED] wrote:
 
  Glen,
  
  I don't
  particularly like selecting renderers by integer
  constant. 
 
 FOP has been using integers for six years now, and as
 I explained earlier [1], MIME types don't make a very
 good primary key for renderers, because not all
 renderers have a MIME type, also multiple renderers
 can share the same MIME type (e.g. our PDF renderer
 and Finn's version of it.).  In some cases, we would
 may indeed need a RENDER_PDF_V14 and an
 RENDER_PDF_V15, for example. 

I would disagree with that. There are ways to do all that pretty cleanly.
The point is that with your approach you always (!) have to work in Java
code to use a non-standard renderer. It can't be plugged in from the
command-line for example.

 Again, integer constants also work well in arrays,
 they are ultra-fast, and, as a bonus, new
 Fop(RENDER_PFD) is a ultra-quick-to-catch
 compile-time error, while new FOP(application/pfd)
 would turn into a pesky run-time error (and even a ML
 question).  The integer constants are also much easier
 to remember than MIME types:  RENDER_ followed by the
 desired render type. 

Performance is really unimportant when we're talking about choosing the
renderer. The compile-time checking argument is valid, however.

 [1]
 http://marc.theaimsgroup.com/?l=fop-devm=109261374110951w=2
 
  I like
  pluggability. 
 
 We sufficiently have that for our next release, where
 I define sufficiently as as much as we have already
 in 0.20.5, and basically what AH/RX offers.  We have
 already learned that adding too many hooks,
 interfaces, visitor patterns, *before* the
 layout/renderer work is done, results in FOP never
 getting finished because the code becomes too hard to
 work through.  So let's get the code working first
 (moving the renderers over, whitespace handling, PDF
 bookmarks, layout), *then* put it into Mandarin with
 the advanced API many desire.  

As you like. But this will stay on my personal todo list.

 I share your impatience with the next release not
 coming out yet, but we have to keep the code simple to
 get more people to look at and help in the code.

I agree, but IMHO it's confusing that we're choosing standard renderers
in one place (Fop.java) but provide a possibility to set non-standard
renderer in another (FOUserAgent.java).

  I'd prefer to register all our
  renderers in a central
  registry. Integrators could plug in their renderers
  using the service
  interface just as the FOP extension can. 
 
 That's beautiful post-0.30/1.0 talk that should be
 added to our roadmap.  But, in the meantime, we
 already have a perfectly satisfactory
 FOUserAgent.setRendererOverride() that will satisfy
 the users that currently have renderer overrides.  I
 would prefer a minimal API with as much black-boxed as
 possible to allow future implementors as much
 architectural freedom as possible.
 
 Besides, what you prefer now is not what the team
 preferred six months ago, or a year ago, or 18 months
 ago, etc., etc.  Our API/internal architecture desires
 keep changing. 

Are they? Is that how you justify ignoring API discussions held earlier?

  You could
  even override
  renderers (for application/postscript for example)
 
 We already have that with
 setRendererOverride(Renderer).  You can't use MIME
 type for reasons given above.

I can't override a Renderer without changing Java code.

  if you have a
  renderer with some custom extensions. IMO
  FOUserAgent is already
  overloaded with too many different things. 
 
 That is because we tend to, whenever any user might
 want something, give him/her an API method for it. 
 The only way we can satisfy these many needs without
 forever locking the FOP architecture (each of these
 switches internally modifying the behavior of one
 internal class or another) is to add the method to
 FOUserAgent, and have internal classes read it, rather
 than the other way around.  Furthermore, we have one
 class--FOUserAgent--for these parameters to make the
 embedded/servlet API easier for non-advanced users. 
 (BTW, FOUserAgent should be reduced somewhat anyway,
 when we create fox:metadata or whatever.)

Means keep Fop.java as lean as possible but no problem with FOUserAgent
getting bloated. I don't get it.

  I think
  that the overrides
  don't belong there, or rather they are probably
  unnecessary given a
  better renderer selection mechanism.
  
 
 Oh, I think they do, it is sufficient for our next
 release.  Nothing is easier, nothing is simpler for
 embedded/servlet users.  Take a look at the 1.0
 embedded examples.

My desires for the API don't complicate things for the embedded examples.

  What triggered the creation of the RendererFactory
  was not IoC but the
  other Avalon concept: Separation of concerns. I
  think that handling
  renderer and FOEventHandler creation in a separate
  class rather than in
  the quite crowded FOTreeBuilder and 

Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java

2004-10-11 Thread Jeremias Maerki
Glen,

I don't want a RendererFactory.setRendererOverride(). I don't
particularly like selecting renderers by integer constant. I like
pluggability. I'd prefer to register all our renderers in a central
registry. Integrators could plug in their renderers using the service
interface just as the FOP extension can. You could even override
renderers (for application/postscript for example) if you have a
renderer with some custom extensions. IMO FOUserAgent is already
overloaded with too many different things. I think that the overrides
don't belong there, or rather they are probably unnecessary given a
better renderer selection mechanism.

What triggered the creation of the RendererFactory was not IoC but the
other Avalon concept: Separation of concerns. I think that handling
renderer and FOEventHandler creation in a separate class rather than in
the quite crowded FOTreeBuilder and RenderPagesModel improves code
readability.

Don't get me wrong. I can live with the current situation as long as I
can plug in my custom FOEventHandler. You don't object to that, do you?
The RendererFactory simply came out of the fact that I need to plug in
my FOEventHandler and I found that the code was IMHO a bit unclean.

By the way, my change didn't change the end-user API, so I don't see any
reason why RendererFactory is a bad idea. Do I miss your point?

On 11.10.2004 01:46:54 Glen Mazza wrote:
 Jeremias,
 
 The reason for getFOEventHandlerOverride()/getRendererOverride() in 
 FOUserAgent is to better black box the FOP system.  This gives us a ton 
 of freedom internally of where we implement FOEventHandlers and 
 Renderers inside FOP.  This has been a perfect example:  So far over the 
 past few months, we've been setting the renderer in apps.Driver(), then 
 in fo.FOTreeControl, then in area.AreaTreeHandler, and then in 
 RenderPagesModel, and now RendererFactory.  But note that during all 
 these movements, FOUserAgent.getRendererOverride() never had to change!  
 I don't know if this is what is meant by Avalon's Inversion of Control, 
 or if this is a separate concept, but it is providing us wonderful 
 flexibility.
 
 If, instead,  we provided a user interface that had the outside user 
 *directly* call RendererFactory.setRendererOverride(), we would be 
 forever stuck with having this code in a RendererFactory class.  Good 
 for you, but some preferred it in AreaTreeModel, and some wanted it in 
 apps.Document or apps.Driver.  Point is, future committers, because of 
 backwards compatibility issues, would be forever frozen with a single 
 design.


Jeremias Maerki



Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java

2004-10-11 Thread Glen Mazza
--- Jeremias Maerki [EMAIL PROTECTED] wrote:

 Glen,
 
 I don't
 particularly like selecting renderers by integer
 constant. 

FOP has been using integers for six years now, and as
I explained earlier [1], MIME types don't make a very
good primary key for renderers, because not all
renderers have a MIME type, also multiple renderers
can share the same MIME type (e.g. our PDF renderer
and Finn's version of it.).  In some cases, we would
may indeed need a RENDER_PDF_V14 and an
RENDER_PDF_V15, for example. 

Again, integer constants also work well in arrays,
they are ultra-fast, and, as a bonus, new
Fop(RENDER_PFD) is a ultra-quick-to-catch
compile-time error, while new FOP(application/pfd)
would turn into a pesky run-time error (and even a ML
question).  The integer constants are also much easier
to remember than MIME types:  RENDER_ followed by the
desired render type. 

[1]
http://marc.theaimsgroup.com/?l=fop-devm=109261374110951w=2

 I like
 pluggability. 

We sufficiently have that for our next release, where
I define sufficiently as as much as we have already
in 0.20.5, and basically what AH/RX offers.  We have
already learned that adding too many hooks,
interfaces, visitor patterns, *before* the
layout/renderer work is done, results in FOP never
getting finished because the code becomes too hard to
work through.  So let's get the code working first
(moving the renderers over, whitespace handling, PDF
bookmarks, layout), *then* put it into Mandarin with
the advanced API many desire.  

I share your impatience with the next release not
coming out yet, but we have to keep the code simple to
get more people to look at and help in the code.


 I'd prefer to register all our
 renderers in a central
 registry. Integrators could plug in their renderers
 using the service
 interface just as the FOP extension can. 

That's beautiful post-0.30/1.0 talk that should be
added to our roadmap.  But, in the meantime, we
already have a perfectly satisfactory
FOUserAgent.setRendererOverride() that will satisfy
the users that currently have renderer overrides.  I
would prefer a minimal API with as much black-boxed as
possible to allow future implementors as much
architectural freedom as possible.

Besides, what you prefer now is not what the team
preferred six months ago, or a year ago, or 18 months
ago, etc., etc.  Our API/internal architecture desires
keep changing. 


 You could
 even override
 renderers (for application/postscript for example)

We already have that with
setRendererOverride(Renderer).  You can't use MIME
type for reasons given above.

 if you have a
 renderer with some custom extensions. IMO
 FOUserAgent is already
 overloaded with too many different things. 

That is because we tend to, whenever any user might
want something, give him/her an API method for it. 
The only way we can satisfy these many needs without
forever locking the FOP architecture (each of these
switches internally modifying the behavior of one
internal class or another) is to add the method to
FOUserAgent, and have internal classes read it, rather
than the other way around.  Furthermore, we have one
class--FOUserAgent--for these parameters to make the
embedded/servlet API easier for non-advanced users. 
(BTW, FOUserAgent should be reduced somewhat anyway,
when we create fox:metadata or whatever.)

 I think
 that the overrides
 don't belong there, or rather they are probably
 unnecessary given a
 better renderer selection mechanism.
 

Oh, I think they do, it is sufficient for our next
release.  Nothing is easier, nothing is simpler for
embedded/servlet users.  Take a look at the 1.0
embedded examples.

 What triggered the creation of the RendererFactory
 was not IoC but the
 other Avalon concept: Separation of concerns. I
 think that handling
 renderer and FOEventHandler creation in a separate
 class rather than in
 the quite crowded FOTreeBuilder and RenderPagesModel
 improves code
 readability.

FOTreeBuilder and RenderPagesModel IMO aren't that
crowded, they are the size they need to be for the
business logic they are responsible for.  Still, I've
bent over backwards to remove as many classes and
unneeded methods to simplify the code as much as
possible.  Furthermore, bouncing around classes IMO
raises stress rather than lowering it.  

As for Avalon SoC, the concerns *were* properly
separated:  the FOTreeBuilder chose the FOEventHandler
based on the desired render type, and RenderPagesModel
chooses its renderer also on the desired render type. 
It is business logic purely the departments of the
FOTreeBuilder and RenderPagesModel.

Still, I'm not objecting to this change as long as
it's not the external API.  (Indeed, I support it,
because it makes you happy, and when you're happy
you're more productive.)

 
 Don't get me wrong. I can live with the current
 situation as long as I
 can plug in my custom FOEventHandler. You don't
 object to that, do you?

Not at all, because it does not directly touch
internal code.  Future 

Re: cvs commit: xml-fop/src/java/org/apache/fop/apps FOUserAgent.java

2004-10-10 Thread Glen Mazza
Jeremias,
The reason for getFOEventHandlerOverride()/getRendererOverride() in 
FOUserAgent is to better black box the FOP system.  This gives us a ton 
of freedom internally of where we implement FOEventHandlers and 
Renderers inside FOP.  This has been a perfect example:  So far over the 
past few months, we've been setting the renderer in apps.Driver(), then 
in fo.FOTreeControl, then in area.AreaTreeHandler, and then in 
RenderPagesModel, and now RendererFactory.  But note that during all 
these movements, FOUserAgent.getRendererOverride() never had to change!  
I don't know if this is what is meant by Avalon's Inversion of Control, 
or if this is a separate concept, but it is providing us wonderful 
flexibility.

If, instead,  we provided a user interface that had the outside user 
*directly* call RendererFactory.setRendererOverride(), we would be 
forever stuck with having this code in a RendererFactory class.  Good 
for you, but some preferred it in AreaTreeModel, and some wanted it in 
apps.Document or apps.Driver.  Point is, future committers, because of 
backwards compatibility issues, would be forever frozen with a single 
design.

Thanks,
Glen
[EMAIL PROTECTED] schrieb:
jeremias2004/10/10 05:24:03
 Modified:src/java/org/apache/fop/fo FOTreeBuilder.java
  src/java/org/apache/fop/area RenderPagesModel.java
  src/java/org/apache/fop/apps FOUserAgent.java
 Added:   src/java/org/apache/fop/render RendererFactory.java
 Log:
 Centralized Renderer and FOEventHandler creation in the RenderFactory class.
 Provide a similar mechanism as for the Renderers to override the FOEventHandler being used to be able to plug in custom FOEventHandlers. (I don't particularly like this approach but so be it for the moment.)
 Javadocs updates in FOUserAgent.
 
 Revision  ChangesPath
 1.1  xml-fop/src/java/org/apache/fop/render/RendererFactory.java
 
 Index: RendererFactory.java
 ===
 /*
  * Copyright 2004 The Apache Software Foundation.
  * 
  * Licensed under the Apache License, Version 2.0 (the License);
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  * 
  *  http://www.apache.org/licenses/LICENSE-2.0
  * 
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an AS IS BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 
 /* $Id: RendererFactory.java,v 1.1 2004/10/10 12:24:02 jeremias Exp $ */
  
 package org.apache.fop.render;
 
 import java.io.OutputStream;
 
 //Avalon
 import org.apache.avalon.framework.configuration.Configuration;
 import org.apache.avalon.framework.configuration.ConfigurationException;
 import org.apache.avalon.framework.container.ContainerUtil;
 
 //FOP
 import org.apache.fop.apps.FOPException;
 import org.apache.fop.apps.FOUserAgent;
 import org.apache.fop.area.AreaTreeHandler;
 import org.apache.fop.fo.Constants;
 import org.apache.fop.fo.FOEventHandler;
 import org.apache.fop.render.mif.MIFHandler;
 import org.apache.fop.render.rtf.RTFHandler;
 
 /**
  * Factory for FOEventHandlers and Renderers.
  */
 public class RendererFactory {
 
 /**
  * Creates a Renderer object based on render-type desired
  * @param renderType the type of renderer to use
  * @return the new Renderer instance
  * @throws IllegalArgumentException if an unsupported renderer type was requested
  */
 private static Renderer newInstance(int renderType) throws IllegalArgumentException {
 
 switch (renderType) {
 case Constants.RENDER_PDF:
 return new org.apache.fop.render.pdf.PDFRenderer();
 case Constants.RENDER_AWT:
 return new org.apache.fop.render.awt.AWTRenderer();
 case Constants.RENDER_PRINT:
 return new org.apache.fop.render.awt.AWTPrintRenderer();
 case Constants.RENDER_PCL:
 return new org.apache.fop.render.pcl.PCLRenderer();
 case Constants.RENDER_PS:
 return new org.apache.fop.render.ps.PSRenderer();
 case Constants.RENDER_TXT:
 return new org.apache.fop.render.txt.TXTRenderer();
 case Constants.RENDER_XML:
 return new org.apache.fop.render.xml.XMLRenderer();
 case Constants.RENDER_SVG:
 return new org.apache.fop.render.svg.SVGRenderer();
 default:
 throw new IllegalArgumentException(Invalid renderer type  
 + renderType);
 }
 }
 
 /**
  * Creates a Renderer object based on render-type desired
  * @param userAgent the user agent for access to configuration
  * @param renderType the type of renderer to use
  * @return the new Renderer instance
  *