cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Pier Fumagalli <p...@betaversion.org>
Subject Re: [Kernel2.2] Comments
Date Wed, 31 Mar 2004 12:28:24 GMT

On 31 Mar 2004, at 12:08, Leo Sutic wrote:

> Nice work!

Thanks

> Some quick observations:
>
>  1. @author <a href="http://www.vnunet.com/">VNU Business 
> Publications</a>
>     Is this OK with Apache and VNU?

Defintely OK with VNU (if someone visits our site because of a click on 
the JavaDOC, and then come back, we got one more customer... If you 
guys don't want to see it in there, you can remove it no problems... It 
is in my default Java class template.

>  2. Wire.java
>     /**
>      * <p>Check wether the wire between this {@link Wirings} instance 
> and the
>      * original component is still active.</p>
>      *
>      * <p>When this method returns <b>true</b> all methods called on

> this
>      * instance will be forwarded to the original component instance 
> associated
>      * with this wire.</p>
>
>     You cannot guarantee that. The instance may be redeployed between 
> the call
>     to wired() and the next call.
>
>     No point in having a method whose return value means nothing.

This method will return true until the block instance will not be 
destroyed or replaced by the Deployer, and when this method returns 
false, whoever got this wire should release it and acquire a new one 
(which will be got from the new block instance).

Rember? The idea is never to completely remove blocks until forced to, 
so the Wire instance will still work even if wired() returns false. The 
wire will only be deactivated by either a call to release() or 
dispose() or by a forceful destruction by the deployer.

>  3. Component.java:
>     /**
>      * <p>Contextualize this {@link Component} component instance with 
> the
>      * {@link Wire} through which its caller is accessing it.</p>
>      *
>      * @param wire the {@link Wire} instance associated with this 
> instance.
>      */
>     public void contextualize(Wire wire);
>
>     Does this mean that each component instance only has one wire? I.e.
>     only one client? You may end up with a lot of components if you 
> have
>
>     component A depends on B, C, D, which depends on E, F, G, H...
>
>     Wouldn't it be better to keep this as a ThreadLocal variable that 
> can
>     be accessed by the component? Then you can have multiple clients,
>     but only one client per calling thread (which seems like a 
> reasonable
>     tradeoff).

Yes, each component instance has only one contextualized wire, because 
that instance is its own wire as it is returned to the caller. The 
JavaDOC goes in quite a lot of details on why this is done, and provide 
some examples.

>  4. Wirings.java
>
>     Talks about ((Wiring)myObject).release();
>     Should be ((Wire)myObject).release();
>
>     #include <my/usual/bitching/about/the/wired()/method>

Fixed.

>  5. Configuration.java
>
>     public class Configuration extends ArrayList
>
>     I'd use composition instead...

??? extends AbstractList ???

>  6. Parameters.java
>
>     Get rid of this one. Why can't components accept a hierarchy of
>     configuration nodes as configuration data?

No... Parameters are typed and the framework checks the type of each 
parameter. One of the parameters CAN be a configuration on the other 
hand if in its descriptor the block requires:

<param name="xyz" type="configuration"/>

Other typse might be:

<param name="abc" type="integer"/>
<param name="def" type="boolean"/>
<param name="ghi" type="string"/>
...

The framework will check that parameters required by the block are 
available (specified before deployment) and of the correct type...
So Configuration is MUCH more restricted than Parameters.

>  7. Configurable.java
>
>     Should accept Configuration instead of Parameters. Being able to
>     send in a tree-structure of config info instead of a flat list is
>     useful.

 From the "Configurable" javadoc:

Note that a Parameters parameter can contain an entire  Configuration 
tree, nullifying the requirement of a configuration  method using a 
Configuration

>  8. Block.java
>
>     Love it.

It's the core.

>  9. Identifier.java
>
>     *
> <p><code>protocol://location/path/major.minor(.revision)?</code></p>
>
>     Does this mean that all Blocks are automatically mapped into a URL
> space?

All blocks have a unique identifier. And this list agreed that each 
block identifier will assume the requirements specified in the 
Identifier interface.

	Pier

Mime
View raw message