Chris

I have other issues as well.

* The dependency on Commons-Lang is just to provide HashCodeBuilder and
ToStringBuilder
  I am not convinced we need to introduce another library just to do this

* You use System.out in several places, presumably for debugging.
  Please use Commons-Logging at trace level for this

* You have a main method in these classes; this is not an entry point for
the
  system. Please use JUnit for testing

Finally, the impl of getChildNodes creates new DDBeanImpls() - should it be
doing this? I would have expected to need a factory for the specific
subclasses.

--
Jeremy

> -----Original Message-----
> From: Aaron Mulder [mailto:[EMAIL PROTECTED]
> Sent: Tuesday, August 19, 2003 8:31 AM
> To: [EMAIL PROTECTED]
> Subject: Re: [PATCH] intitial add DDBeanImpl and DDBeanRootImpl - first
> take
>
>
> On Mon, 18 Aug 2003, Chris Opacki wrote:
> > org.apache.geronimo.enterprise.deploy.tool.DDBeanImpl
> > org.apache.geronimo.enterprise.deploy.tool.DDBeanRootImpl
>
> Chris,
>
>       It looks like there are a couple issues here:
>
>  - Commons Lang and Dom4j need to be added to the dependency list for
> these to compile
>
>  - They don't fully implement the JSR-88 interfaces that are currently in
> CVS.  I suspect you're using the 1.0 version of JSR-88, whereas the
> version in CVS includes new methods defined in the update for J2EE 1.4
> (v1.1):
>
> http://java.sun.com/j2ee/tools/deployment/88ChangeLog1.1_aug2802.html
>
>  - The main method has a reference to a specific file name
> (c:\java_specs\...)
>
>       I'll clean this up a bit when I get a chance.
>
> Aaron
>
>

Reply via email to