Greg Reddin wrote:
On Oct 31, 2006, at 3:10 PM, David H. DeWolf wrote:
I'm wondering why the ComponentDefinitions interface has been exposed
outside of the DefinitionsFactory. To me, this class seems like an
implementation detail of the factory itself, and it should not be
exposed.
If you look back at Tiles 1 you'll see that DefinitionsFactory and its
descendants pretty much contained all of the functionality that we've
separated into DefintionsFactory and ComponentDefinitions. It was both
a factory and a container if you will. This was especially true if you
drilled down into xmlDefinitions and the classes under that. A lot of
core Tiles functionality was embedded deep into the XML version of the
implementation and not exposed on the API.
Let's keep in mind the value of separation of concerns. I don't think
we want the factory to do too much. Remember what the purpose of a
factory is - to create objects and nothing more. I think anything
beyond the creation and storage of definitions should be delegated
outside the factory so that if someone wants to override the creation
and storage functionality, but wants to keep other pieces in place they
can do that. See further comments below:
Yes, I agree that we don't want the factory to do too much. That said,
it's already responsible for a lot (creation, caching, locale
resolution, etc. . .). I think that it's really more a manager and
perhaps we're getting caught up in semantics. It already delegates to
the reader for the actual parsing.
1) Encapsulate the refresh logic in the DefinitionsFactory. The
filter will change to:
if(factory.refreshRequired()) {
// replace refresh logic with a call
// to the factory, removing the reference
// to ComponentDefinitions
factory.refresh();
}
I'm OK with this because it still seems related to "factory" like code
to me. The factory is being used for manufacturing and repair in this
case :-) That doesn't bother me.
It's probably ideal to have a Manager of some sort that encapsulates the
caching and refresh logic and delegates to the factory for the actual
"creation" operations. If that's what you're thinking in the back of
your head, I'm on board with it.
The one complexity there is that the refresh logic actually requires a
bit of low level operations that the factory should only be aware of
(specifically, opening the URLConnection). Because of that, I'd tend to
leave the actual refresh determination in the factory, but move the
caching and management of the actual entities up a level.
2) TilesUtilImpl only exposes the ComponentDefinitions in order to
allow the Filter (#1) to access them. This reference can easily be
removed.
This is true, but TilesUtilImpl is likely going to be replaced by our
container API. So maybe the container API replaces
ComponentDefinitions. That's really what ComponentDefinitions was
created for - to separate container logic from creation logic. So, if
the container exposes everything that's currently being taken care of by
ComponentDefinitions I'm cool with it. But, again, I want to avoid a
monolithic API that does too much. We need to find the sweet spot of
APIs that are small and manageable, but yet complete.
I'm thinking that the container literally exposes a few methods as
simple as:
void render(Object request, Object response, String preparer)
That way, clients (and hopefully even our tags) loose a lot of knowledge
about the ComponentDefinitions. Under the covers, the Container will
maintain the ComponentDefinitions and worry about the lifecylce of
invocation (meaning, 1) creating the component context, 2) invoking the
preparer, 3) invoking the definitions, etc. . .)
3) Encapsulate the hierarchy resolution within the DefinitionsFactory,
allowing the resolution to occur during initialization.
Looking at ComponentDefinitions right now, it provides APIs to add
definitions, get definitions, and resolve inheritances (and some
ancillary things that might just be side effects). DefinitionsFactory
has APIs to get and read definitions. There's some overlap, redundancy,
and perhaps misplaced responsibilities. I do think we need to rethink
some things, but I'm not convinced that dumping it all into the factory
is the right thing to do.
Gotcha!
In fact, I didn't mean to imply that we would dump it all in the
factory. Instead, I was thinking incrementally. My first step was to
remove the Factory's knowledge of it's outside world by not publishing
the ComponentDefinitions. I thought that would entail two steps -
having the Filter invoke the factory to do the refresh logic and
resolving inheritance differently. As Antonio pointed out, the
inheritance references in the ComponentDefinition aren't even used
anymore so the change actually turns out to be even simpler.
Since it seems as though we all agree on this first step, why don't we
move forward with it and then I'll take a deeper look into a larger
refactoring.
Maybe we can back up a bit, identify the core responsibilities, and
decide where each one fits between the factory, the container, and
whatever else.
Sounds like a plan.
David
Greg
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
