RE: dom loading in Barracuda - proposed change

Ok, I'm _finally_ getting around to trying to catch up on my email. Sorry
for the delay. My comments follow...

So Jake & Shawn, if I understand things correctly, there are basically 2
issues being discussed:

1. simplify the DOMLoading interface like this: (Jake's request)

> The following methods are now all that exist in DOMLoader....
>      public Document getDOM(String doc) throws IOException;
>      public Document getDOM(String doc, Locale locale) throws IOException;
>      public Document getDOM(Class clazz) throws IOException;
>      public Document getDOM(Class clazz, Locale locale) throws
> IOException;
...
> Notice that the getDOM(String) methods now expect a file/URL path
...
> Also notice that there are no methods taking ViewCapabilities.

I can't really think of any objections to doing this, because as you say -
its only going to implement those who have written their own DOMLoader
implementations, and that's probably not  very many people. My question here
is "are there any other changes beyond those I've just listed above" (I
don't think so, but I want to be sure).


2. modify the DOMWriter class/implementations like this: (Shawn's request)

> My proposal is that we remove the "leaveWriterOpen" option from
> DefaultDOMWriter and provide eplanation in the comments of DOMWriter
> that implementing class MUST NOT close the output. It should be stated
> clearly that it is the responsibility of the class(es) using the
> DOMWriter to manually close any output streams.

Ok, regarding this change, here's some history...

a) originally - the dom writer opened the stream, did the writing, and then
closed the stream (well, it didn't actually open it, but it did create the
Writer that wraps the output stream, set the headers, etc...since it opens
the Writer, I think it makes sense to close the writer as well)

b) I ran into a situation where I needed to have the stream left open
because I wanted to be able to process very large DOMs in blocks, using the
BlockIterator. So the easiest way to handle this was to simply pass a flag
in saying "hey, leave that stream open! (because I'm going to call you again
in just a sec"

c) Shawn's proposal is to clean up the DOMWriter class so that it neither
opens nor closes the writer. I'm ok with this in principle, but I'm also
sympathetic which Jakes objection that it does make it more cumbersome to
use - in the majority of the cases, I think its most intuitive to just use
the DOM writer the way we are: write to it and forget it.

So I guess my big question here is this: what exactly does this change give
us over and above the current implementation?

Whatever we decide, the bottom line is that we need to be able to write to
the DOM writer in 2 different ways - in the first (and by far the most
common), we simply write and forget about it; in the second (rare, but
definitely important), we are going to repeatedly use the writer to write
multiple doms to the same response stream.

Ultimately, as long as we keep the functionality, I'm ok with Shawn's
approach, but I think I'd like to see more tangible value before doing it
(ie. we shouldn't do it just for the sake of doing it).

So, to summarize...
#1 - +1
#2 - unsure until I see more data

That help?

Christian
----------------------------------------------
Christian Cryder
Internet Architect, ATMReports.com
Project Chair, BarracudaMVC - http://barracudamvc.org
----------------------------------------------
"Coffee? I could quit anytime, just not today"


> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]]On Behalf Of Jacob Kjome
> Sent: Wednesday, October 01, 2003 2:41 PM
> To: BarracudaMVC Users List
> Subject: [Barracuda] dom loading in Barracuda - proposed change
>
>
>
> Hi everyone,
>
> Since we now have a simple viable alternative to XMLC DOM Loading via
> Jivan, I thought I'd try it out.  I created a new JivanDOMFactory which
> seems to work nicely.  However, as I was looking at the DOMLoader and
> DefaultDOMLoader, I noticed some unnecessary complexity (partially my
> fault).  I'm wondering if anyone would object to a slight change to the
> interface.  I was thinking of getting rid of any method taking
> ViewCapabilities as a parameter and any non-getDOM() methods.  This would
> simplify the interface significantly and keep more
> implementation-specific
> methods out of the Interface.  This would make other DOMLoader
> implementations much easier to create and maintain.
>
> What I'd like to do is point out the changes to the DOMLoader
> interface and
> where these changes affect existing code.  After evaluating that,
> I'd like
> to hear your point of view whether this change would cause any pain for
> existing applications.  So, here are the proposed changes...
>
> The following methods are now all that exist in DOMLoader....
>      public Document getDOM(String doc) throws IOException;
>      public Document getDOM(String doc, Locale locale) throws IOException;
>      public Document getDOM(Class clazz) throws IOException;
>      public Document getDOM(Class clazz, Locale locale) throws
> IOException;
>
> Notice that the getDOM(String) methods now expect a file/URL path to a
> document rather than being convenience methods to send in a fully
> qualified
> class name as they were before (and were only recently added by
> me).  This
> has replaced getDOMFromFile(String) that had been added to
> support dynamic
> loading of documents in Barracuda-1.2.5.  I don't think this should cause
> undue pain since it is more likely that people use the getDOM(Class)
> methods anyway.
>
> Also notice that there are no methods taking ViewCapabilities.
> The reason
> for this is that we might as well just pass in the Locale since
> that is all
> we use the ViewCapabilities for anyway...at least in dom loading; dom
> writing may be another issue.  The way this affects existing code in
> Barracuda is to change this:
> Document page =
> DefaultDOMLoader.getGlobalInstance().getDOM(clazz,
> vc.getViewCapabilities());
> to this:
> Document page =
> DefaultDOMLoader.getGlobalInstance().getDOM(clazz,
> vc.getViewCapabilities().getClientLocale());
>
> Not too tough, if you ask me.
>
>
> The other methods that have gone away from the DOMLoader interface are...
>
>      public void setDefaultDOMFactory(DOMFactory df);
>      public void registerDOMFactory(DOMFactory df, Class clazz);
>      public void deregisterDOMFactory(Class clazz);
>
> These seem to me to be more implementation-specific.  I've left modified
> versions of them in DefaultDOMLoader, but I think the DOMLoader can live
> with the getDOM() methods alone.  I changed these methods in
> DefaultDOMLoader to take a String instead of a Class to key particular
> DOMFactory's.  This makes it easier to support the keys as being either
> class names or document file paths which allows for registering
> individual
> files to be loaded by, say..., the JivanDOMFactory and individual class
> names by one of the XMLC dom factories.  Before, this was only
> possible for
> XMLC-based classes.
>
>
> Now, besides the previously described necessary change to
> existing code in
> order to be compatible with the new DOMLoader interface, I had to comment
> out all the dom loader stuff in DefaultApplicationAssembler.  I'm
> not sure
> how many people out there currently use the <dom-loader> and/or
> <dom-loader-register> stuff in your application assembler files.
> I suspect
> not too many.  Of course, this functionality has already been
> superceded by
> the object repository assembler anyway.  So, the functionality is still
> there (in an even more flexible form), it just would no longer be
> supported
> in the DefaultApplicationAssembler.
>
>
> Basically, if people have been using the basics for dom loading, I don't
> see this change as affecting people much at all.   I hope that is the
> case.  Please let me know your opinion on what I've described above and
> tell me whether you think this change is acceptable or not.
>
> Jake
>
> _______________________________________________
> Barracuda mailing list
> [email protected]
> http://barracudamvc.org/lists/listinfo/barracuda