struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Reddin <gred...@apache.org>
Subject Re: [tiles2] ComponentDefinitions - exposed implementation detail?
Date Wed, 01 Nov 2006 17:29:21 GMT

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:

> 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.

> 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.

> 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.

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.

Greg


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org


Mime
View raw message