cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Leo Sutic" <leo.su...@inspireinfrastructure.com>
Subject [Kernel2.2] Comments
Date Wed, 31 Mar 2004 11:08:21 GMT
Nice work!

Some quick observations:

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

 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.

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

 4. Wirings.java

    Talks about ((Wiring)myObject).release();
    Should be ((Wire)myObject).release();

    #include <my/usual/bitching/about/the/wired()/method>

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

 6. Parameters.java

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

 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.

 8. Block.java
 
    Love it.

 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?

Still trying to get my head around the rest.

/LS


Mime
View raw message