Checking for OutputSource for renderer overrides

[Glen Mazza <gr...@yahoo.com>]

Jeremias,

Per your change here [1], no longer checking for an
OutputStream variable if the Renderer is overridden:
The render constant chosen is irrelevant when the
renderer is overridden.  So the embedded programmer
can rely on RENDER_AWT or _PRINT for those
comparatively rare cases of not needing an
OutputStream (and, indeed, that would be the
appropriate render type in most of those instances
anyway.) 

People will be supplying an OutputSource for PDF-type
generation--FOP has been requiring it for six years
now with nary a complaint.  If you remove that check,
and they forget to programmatically supply an output
stream (a fairly common newbie mistake), errors in FOP
will occur without informative messages.  

So I would advise the usage of RENDER_AWT/_PRINT for
non-OutputStream usage for overridden renderers, while
returning the check for the RENDER_PDF et al ones. 
That should be sufficient for our next release, after
which we can then consider "what's next" based on user
feedback.

Glen

[1]
http://marc.theaimsgroup.com/?l=fop-cvs&m=110495921529754&w=2

Re: Checking for OutputSource for renderer overrides

[Jeremias Maerki <de...@greenmail.ch>]

On 08.01.2005 19:27:38 Glen Mazza wrote:
> --- Jeremias Maerki <de...@greenmail.ch> wrote:

<snip/>

> > If someone embeds FOP in
> > his/her application the
> > not supplying an OutputStream when a renderer needs
> > one is a bug and so
> > it doesn't matter if the error message comes early
> > or only when the
> > renderer is started (Renderer.startRenderer()).
> > 
> 
> OK, I see what you're saying here--it would be one
> time only programmatic change for the user to fix his
> embedded code.

Yes.

> If I understand you correctly, all that would be
> needed for renderer overrides is to just raise an
> exception within Renderer.startRenderer(OutputStream)
> if the OutputStream is null (for those renderers
> requiring an OutputStream).  Arguably, the Renderer
> should be programmed to do that anyway, even if it is
> checked earlier.

Yes.

>  That makes sense--and I now agree
> with your previous change of not checking for an
> OutputStream when the RendererOverride is set.  Thanks
> for the clarification.
> 
> (I still prefer the one-line check in RendererFactory
> for our *hardcoded* renderers--for
> documentation/comprehension reasons, and it gives us a
> common error message for our own renderers.)

That's ok. I'll do the necessary changes this week.

<snip/>

> > So here's another one. Do you know about SVG Print?
> > An SVG renderer
> > could send the generated SVG using SAX. That way a
> > developer could run
> > the generated SVG through an XSLT post-processing
> > without having to
> > reparse the generated SVG. That's a place where
> > speed is very important
> > and an OutputStream-only system would be suboptimal.
> > 
> 
> I don't completely understand this idea (Namely, how
> can you send "generated SVG using SAX"--wouldn't that
> mean the "generated SVG" would have to be in XML
> format?)--but it doesn't matter anyway because, again,
> I don't have a problem with your validation change.  

"sending generated SVG using SAX" means that all the XML elements that
are generated by the SVG Renderer would be sent as start/endElement
calls through a ContentHandler interface. I'm doing the same for the
ExampleObj2XML class in the embedding examples, only that it's plain
custom XML there. You can catch these SAX events with a JAXP
Transformer(Handler) and do anything with them: simple serialization to
streamed XML (StreamResult), build a DOM (DOMResult), transform using
XSLT, whatever.


Jeremias Maerki

Re: Checking for OutputSource for renderer overrides

[Glen Mazza <gr...@yahoo.com>]

--- Jeremias Maerki <de...@greenmail.ch> wrote:

> I think you're confusing things. 

Yes, I think I was.

> If someone embeds FOP in
> his/her application the
> not supplying an OutputStream when a renderer needs
> one is a bug and so
> it doesn't matter if the error message comes early
> or only when the
> renderer is started (Renderer.startRenderer()).
> 

OK, I see what you're saying here--it would be one
time only programmatic change for the user to fix his
embedded code.

If I understand you correctly, all that would be
needed for renderer overrides is to just raise an
exception within Renderer.startRenderer(OutputStream)
if the OutputStream is null (for those renderers
requiring an OutputStream).  Arguably, the Renderer
should be programmed to do that anyway, even if it is
checked earlier.  That makes sense--and I now agree
with your previous change of not checking for an
OutputStream when the RendererOverride is set.  Thanks
for the clarification.

(I still prefer the one-line check in RendererFactory
for our *hardcoded* renderers--for
documentation/comprehension reasons, and it gives us a
common error message for our own renderers.)


> 
> Ok, here's your technical justification:
> I'm certain you've seen the new test subsystem for
> the layout engine
> where I analyze the output of the area tree
> renderer. If I wrote the
> area tree to an OutputStream I'd have to parse it
> again later, so I get
> a DOM I can evaluate XPath statements on. If I can
> pass in a
> TransformerHandler into the XMLRenderer the renderer
> can simply send SAX
> events which are used by a Transformer to build a
> DOM from (directly).
> 

I haven't looked at your test framework yet here--I
hope to be able to do so sometime soon.  I'm sure it's
very good.  I still have a little bit more bookmark
stuff to do.


> So here's another one. Do you know about SVG Print?
> An SVG renderer
> could send the generated SVG using SAX. That way a
> developer could run
> the generated SVG through an XSLT post-processing
> without having to
> reparse the generated SVG. That's a place where
> speed is very important
> and an OutputStream-only system would be suboptimal.
> 

I don't completely understand this idea (Namely, how
can you send "generated SVG using SAX"--wouldn't that
mean the "generated SVG" would have to be in XML
format?)--but it doesn't matter anyway because, again,
I don't have a problem with your validation change.  

Thanks,
Glen

Re: Checking for OutputSource for renderer overrides

[Glen Mazza <gr...@yahoo.com>]

--- Jeremias Maerki <de...@greenmail.ch> wrote:
> > 
> > Personally, I would rather we get rid of
> > RendererFactory and put the logic back where it
> > was--in FOTreeBuilder and RenderPagesModel.  This
> > functionality is just too specific to be reused
> > elsewhere in FOP.
> 
> I don't see your point and you were the only one who
> complained about
> the RendererFactory. I still think the
> RendererFactory is A Good Thing 
> (tm). It reduces imports, dependencies and number of
> lines in
> FOTreeBuilder and RenderPagesModel....

I don't see how adding a class in the renderer
package, which fo.FOTreeBuilder and
area.RenderPagesModel now must access in order to do
their work, reduces the number of dependencies and
imports.  (Indeed, we now have a new dependency: 
RendererFactory.)

Also, the fact that the new solution has more classes
and more overall LOC would appear to invalidate the
benefit of there being fewer LOC in FOTB and RPM as a
result. 

Usually for a factory pattern, its biggest selling
point is in reuse--i.e., centralizing certain logic so
it doesn't have to be duplicated in multiple places. 
But there is no reuse being realized here.  (Perhaps
you see some in the future however.)


> and centralizes
> instantiation of the
> Renderers and FOEventHandler in one place where they
> are easier to find
> for those unfamiliar with FOP sources.
> 

True--but is that an acceptable OO design?  Can we
indeed just rip out disparate business logic from
various classes and place them into one class for
convenience?  Is there really an object that would
know which FOEventHandler an FOTreeBuilder requires
*and* which Renderer the RenderPagesModel needs? 
(They are different issues, after all.)

RendererFactory seems to be a C-language-like non-OO
collection of business logic, an artificial object,
and the fact that it has no class or instance
variables would appear to add to that argument.

Still, this is just one additional class so hardly an
architecture-breaker.  (And I can see the convenience
regardless of having them together in one place.)  So
just take this posting as a desire on my part to
further clarify my concerns about this class.

Thanks,
Glen

Re: Checking for OutputSource for renderer overrides

[Jeremias Maerki <de...@greenmail.ch>]

On 07.01.2005 11:01:28 Glen Mazza wrote:
> --- Jeremias Maerki <de...@greenmail.ch> wrote:
> 
> > You're right, my change was suboptimal. When I think
> > about this I must
> > say that I'd prefer to remove that check entirely
> > and let the individual
> > renderers check if they have everything to write to
> > their target. 
> 
> I don't think so, because we need early checking at
> the command line and embedded usage to make sure all
> input is OK before proceeding.  That's a very common
> paradigm used in just every compiler or text
> processor.  FOP has been doing this for several years.

I think you're confusing things. Checking for the availability of an
OutputStream has absolutely nothing to do with the command line. Command
line parameter validation is the responsibility of the
CommandLineOptions class. If through the command line no OutputStream is
set it's a bug in FOP. If someone embeds FOP in his/her application the
not supplying an OutputStream when a renderer needs one is a bug and so
it doesn't matter if the error message comes early or only when the
renderer is started (Renderer.startRenderer()).

> > The
> > renderer knows best what it needs. 
> 
> I don't think so--all of the non-AWT based renderers,
> for the past seven years, require a OutputSource and
> only an OutputSource.  We never had a user complaint
> on that or a request otherwise for a change.  It has
> never been a problem.

You mean OutputStream, not OutputSource, don't you?

> BTW, you are glossing over how one can generate a PDF
> or any other output document without using an
> OutputSource.  Absent user demand or a technical
> explanation from you on this point, I can't really
> support your position.

Ok, here's your technical justification:
I'm certain you've seen the new test subsystem for the layout engine
where I analyze the output of the area tree renderer. If I wrote the
area tree to an OutputStream I'd have to parse it again later, so I get
a DOM I can evaluate XPath statements on. If I can pass in a
TransformerHandler into the XMLRenderer the renderer can simply send SAX
events which are used by a Transformer to build a DOM from (directly).

Ok, this is no end-user use case but it's still a use case. You could
even argue that speed is no big issue in this case. Granted.

So here's another one. Do you know about SVG Print? An SVG renderer
could send the generated SVG using SAX. That way a developer could run
the generated SVG through an XSLT post-processing without having to
reparse the generated SVG. That's a place where speed is very important
and an OutputStream-only system would be suboptimal.

> > Having this check
> > in the
> > RendererFactory only puts renderer-dependant logic
> > in a place that is
> > renderer-agnostic. If it's ok by you I'm going to
> > fix that.
> > 
> 
> Personally, I would rather we get rid of
> RendererFactory and put the logic back where it
> was--in FOTreeBuilder and RenderPagesModel.  This
> functionality is just too specific to be reused
> elsewhere in FOP.

I don't see your point and you were the only one who complained about
the RendererFactory. I still think the RendererFactory is A Good Thing 
(tm). It reduces imports, dependencies and number of lines in
FOTreeBuilder and RenderPagesModel and centralizes instantiation of the
Renderers and FOEventHandler in one place where they are easier to find
for those unfamiliar with FOP sources.

I'll gladly submit to a veto against the RendererFactory if it is
supported by at least another committer.

> > I see two possibilities: Adding a RENDER_CUSTOM
> > constant or having a Fop
> > contructor without this constant.
> > 
> 
> RENDER_CUSTOM is OK.  (But there can also be
> advantages of still encouraging users to use
> RENDER_PDF, for example, if the override is still a
> PDF renderer.  e.g., checking for an output source or
> other business logic.)

Ok.

> > Please keep in mind that if you simply define a new
> > API, release and
> > then change according to user feedback you can't
> > just change the API in
> > an incompatible way again. You'd have to provide
> > API-compatibility
> > within smaller version steppings. See for example
> > [1].
> > 
> 
> Indeed, this reason is why I'm against adding API
> methods without an explanation of the technical needs
> for them, or bothering to point to user demand or
> requests for them first.  Once it's there, we're
> forever stuck with it unless we do another major
> release.  So I think you should be keeping versioning
> etc. in mind as well.
> 
> This is also the reason why we are using a simple
> JAXP-based API--something we know that we will support
> now and forever.  (Adding to an API is always OK, as
> more functionality and enhancements are provided--it
> is *subtracting* from it that causes the problem.) 
> Additional functionality should be based on user
> requests and needs, not every possible theoretical
> usage.



Jeremias Maerki

Re: Checking for OutputSource for renderer overrides

[Glen Mazza <gr...@yahoo.com>]

--- Jeremias Maerki <de...@greenmail.ch> wrote:

> You're right, my change was suboptimal. When I think
> about this I must
> say that I'd prefer to remove that check entirely
> and let the individual
> renderers check if they have everything to write to
> their target. 

I don't think so, because we need early checking at
the command line and embedded usage to make sure all
input is OK before proceeding.  That's a very common
paradigm used in just every compiler or text
processor.  FOP has been doing this for several years.

> The
> renderer knows best what it needs. 

I don't think so--all of the non-AWT based renderers,
for the past seven years, require a OutputSource and
only an OutputSource.  We never had a user complaint
on that or a request otherwise for a change.  It has
never been a problem.

BTW, you are glossing over how one can generate a PDF
or any other output document without using an
OutputSource.  Absent user demand or a technical
explanation from you on this point, I can't really
support your position.


> Having this check
> in the
> RendererFactory only puts renderer-dependant logic
> in a place that is
> renderer-agnostic. If it's ok by you I'm going to
> fix that.
> 

Personally, I would rather we get rid of
RendererFactory and put the logic back where it
was--in FOTreeBuilder and RenderPagesModel.  This
functionality is just too specific to be reused
elsewhere in FOP.


> I see two possibilities: Adding a RENDER_CUSTOM
> constant or having a Fop
> contructor without this constant.
> 

RENDER_CUSTOM is OK.  (But there can also be
advantages of still encouraging users to use
RENDER_PDF, for example, if the override is still a
PDF renderer.  e.g., checking for an output source or
other business logic.)

> Please keep in mind that if you simply define a new
> API, release and
> then change according to user feedback you can't
> just change the API in
> an incompatible way again. You'd have to provide
> API-compatibility
> within smaller version steppings. See for example
> [1].
> 

Indeed, this reason is why I'm against adding API
methods without an explanation of the technical needs
for them, or bothering to point to user demand or
requests for them first.  Once it's there, we're
forever stuck with it unless we do another major
release.  So I think you should be keeping versioning
etc. in mind as well.

This is also the reason why we are using a simple
JAXP-based API--something we know that we will support
now and forever.  (Adding to an API is always OK, as
more functionality and enhancements are provided--it
is *subtracting* from it that causes the problem.) 
Additional functionality should be based on user
requests and needs, not every possible theoretical
usage.

Thanks,
Glen

Re: Checking for OutputSource for renderer overrides

[Jeremias Maerki <de...@greenmail.ch>]

You're right, my change was suboptimal. When I think about this I must
say that I'd prefer to remove that check entirely and let the individual
renderers check if they have everything to write to their target. The
renderer knows best what it needs. Having this check in the
RendererFactory only puts renderer-dependant logic in a place that is
renderer-agnostic. If it's ok by you I'm going to fix that.

Setting the OutputStream on the Fop class is merely a convenience method
that simplifies setup for normal cases. There are simply cases where no
OutputStream is needed. 

What I don't like is using RENDER_AWT or RENDER_PRINT just to specify
that I don't use an OutputStream. That's very unintuitive and misleading.
I see two possibilities: Adding a RENDER_CUSTOM constant or having a Fop
contructor without this constant.

Please keep in mind that if you simply define a new API, release and
then change according to user feedback you can't just change the API in
an incompatible way again. You'd have to provide API-compatibility
within smaller version steppings. See for example [1].

[1] http://jakarta.apache.org/commons/releases/versioning.html

On 07.01.2005 01:19:36 Glen Mazza wrote:
> Jeremias,
> 
> Per your change here [1], no longer checking for an
> OutputStream variable if the Renderer is overridden:
> The render constant chosen is irrelevant when the
> renderer is overridden.  So the embedded programmer
> can rely on RENDER_AWT or _PRINT for those
> comparatively rare cases of not needing an
> OutputStream (and, indeed, that would be the
> appropriate render type in most of those instances
> anyway.) 
> 
> People will be supplying an OutputSource for PDF-type
> generation--FOP has been requiring it for six years
> now with nary a complaint.  If you remove that check,
> and they forget to programmatically supply an output
> stream (a fairly common newbie mistake), errors in FOP
> will occur without informative messages.  
> 
> So I would advise the usage of RENDER_AWT/_PRINT for
> non-OutputStream usage for overridden renderers, while
> returning the check for the RENDER_PDF et al ones. 
> That should be sufficient for our next release, after
> which we can then consider "what's next" based on user
> feedback.
> 
> Glen
> 
> [1]
> http://marc.theaimsgroup.com/?l=fop-cvs&m=110495921529754&w=2



Jeremias Maerki