on 4/17/00 10:01 PM, Chris Meyer <[EMAIL PROTECTED]> wrote:
>> Chris, how about sending me a URL for a complete patch?
>
> (If nothing else, read the last two paragraphs...)
>
> I've been looking at the code in order to make the patch; but I've
> come across some items that I think I should bring up...
Ok...
> Basically, some of the 'Assemblers' treat the 'build' method in one
> of two ways:
>
> 1 - construct the page and return it as a concrete element
> 2 - store the page into the document created when 'RunData.getPage' is invoked
Correct. There is also a 3rd short circuit that allows you to write directly
to the servletoutputstream. See the Screen "ImageServer" for an example of
why this is necessary.
> Examples of assemblers that follow this model are 'Screen' and
> 'Navigation' and probably others.
>
> Other Assemblers treat build only in the second way:
>
> - store the page into the document created when 'RunData.getPage' is invoked
>
> Examples of assemblers following this model are 'Page' and 'Layout'.
>
> I think this multi-technique facet of 'build' will lead to problems
> in the future. Since RunData is an object that is passed around to
> lots of things throughout the code, one can almost treat it as a
> global variable and I tend to think that storing data into that
> object will lead to maintenance problems down the road. I would think
> the better technique would be to always return a Concrete object. If
> one wanted to chain together multiple objects one could always create
> a 'MultiScreen' class to do that...
RunData is NOT global. Well, it is in the perspective of a single invocation
of Turbine, but it is unique across invocations. It isn't and should not be
cached or have stored as data ANYWHERE.
> Is the multiple definitions of 'build' intentional or a result of
> migrating code? Is anyone else concerned about this? Is anyone not
> concerned? Should we try to do something to clear up the definition a
> bit?
It is intentional....the modules all subclass Assemblers, right? :-)
The intentional behavior may or may not be "right", but I did work it that
way because Modules are essentially the same but are re-purposed and
extended for their own specific needs. ie: Layout is not the same as Screen,
but it is close.
> Along the same lines, the RunData.getPage() method worries me a
> little. It has a non-trivial side-effect which is to create a page
> and mark the RunData as 'having a page' in the variable 'isPageSet'.
> Usually 'get' methods don't have side effects within the class...
> Does anyone else know more about this than me?
I do. I implemented it.
Essentially, I wanted to make thing easier for the Module creator by
providing a single instance of Document for people to just stuff their data
into without having to worry about whether it existed or not. getPage() does
that for you...all you care about is that you get a valid Document object
back. A factory model would have probably been good here except that we are
not generating multiple Document's...only one.
The isPageSet() stuff is the short circuit hack that I added so that people
could just grab a ServletOutputStream and write to it directly. If you look
in Turbine.java, that is the only place where this stuff is really needed
because if someone has grabbed an OutputStream, then we don't want to also
write a Document out with it...it prevents Module authors from screwing up
accidentally.
> Is anyone else worried about stuff like this? As a new, serious user
> of Turbine, items like this concern me. Is this the type of stuff
> that is still useful to discuss in this project or have these
> decisions already been hashed out? I would like to work on Turbine
> and make it an industrial strength framework that professional sites
> use out of the box. Towards that end, it seems to me that discussions
> like this need to take place... Any opinions?
You are worried a lot more about the core of the framework and naming
conventions than I really would be. Sure, some of that stuff can be cleaned
up, but it does work quite nicely and 99% of the Module authors out there
won't even notice it...
You are also sounding frustrated at this point...I suggest that you take a
break from this stuff for a bit because you really shouldn't get frustrated
over something like this...if you want to help us improve it, then that is
one thing...just don't get frustrated and take it out on this list please.
:-)
> Any responses would be greatly appreciated as it will allow me to
> focus my efforts in the appropriate places...
I would much rather prefer that you focus on other things than worrying
about what the names of things are, but that seems to be important to you
and I respect that...obviously John and Frank do as well because we gave you
a +1 on a pretty major core change...send us the patch so that we can make
it happen...
-jon
--
Scarab -
Java Servlet Based - Open Source
Bug/Issue Tracking System
<http://scarab.tigris.org/>
------------------------------------------------------------
To subscribe: [EMAIL PROTECTED]
To unsubscribe: [EMAIL PROTECTED]
Problems?: [EMAIL PROTECTED]
