Re: Checking for OutputSource for renderer overrides
--- Jeremias Maerki <[EMAIL PROTECTED]> 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
On 08.01.2005 19:27:38 Glen Mazza wrote: > --- Jeremias Maerki <[EMAIL PROTECTED]> wrote: > > 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. > > 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
--- Jeremias Maerki <[EMAIL PROTECTED]> 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
On 07.01.2005 11:01:28 Glen Mazza wrote: > --- Jeremias Maerki <[EMAIL PROTECTED]> 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
--- Jeremias Maerki <[EMAIL PROTECTED]> 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
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