Return-Path: 
 <commons-dev-return-56025-apmail-jakarta-commons-dev-archive=jakarta.apache.org@jakarta.apache.org>
Delivered-To: apmail-jakarta-commons-dev-archive@www.apache.org
Received: (qmail 34419 invoked from network); 25 Jul 2004 19:12:11 -0000
Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199)
  by minotaur-2.apache.org with SMTP; 25 Jul 2004 19:12:11 -0000
Received: (qmail 62006 invoked by uid 500); 25 Jul 2004 19:12:10 -0000
Delivered-To: apmail-jakarta-commons-dev-archive@jakarta.apache.org
Received: (qmail 61690 invoked by uid 500); 25 Jul 2004 19:12:08 -0000
Mailing-List: contact commons-dev-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
List-Unsubscribe: <mailto:commons-dev-unsubscribe@jakarta.apache.org>
List-Subscribe: <mailto:commons-dev-subscribe@jakarta.apache.org>
List-Help: <mailto:commons-dev-help@jakarta.apache.org>
List-Post: <mailto:commons-dev@jakarta.apache.org>
List-Id: "Jakarta Commons Developers List" <commons-dev.jakarta.apache.org>
Reply-To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
Delivered-To: mailing list commons-dev@jakarta.apache.org
Received: (qmail 61677 invoked by uid 99); 25 Jul 2004 19:12:08 -0000
X-ASF-Spam-Status: No, hits=1.4 required=10.0
	tests=DNS_FROM_RFC_ABUSE,DNS_FROM_RFC_POST
X-Spam-Check-By: apache.org
Received: from [217.12.12.141] (HELO smtp804.mail.ukl.yahoo.com)
 (217.12.12.141)
  by apache.org (qpsmtpd/0.27.1) with SMTP; Sun, 25 Jul 2004 12:12:04 -0700
Received: from unknown (HELO oemcomputer)
 (commons-dev@jakarta.apache.org@217.43.117.214 with poptime)
  by smtp804.mail.ukl.yahoo.com with SMTP; 25 Jul 2004 19:12:01 -0000
Message-ID: <000e01c4727b$82dbc000$d6752bd9@oemcomputer>
From: "Stephen Colebourne" <scolebourne@btopenworld.com>
To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
References: <000701c43a0d$c201cbe0$c9409a51@oemcomputer>
 <4103F84A.1050208@steitz.com>
Subject: Re: [math] Design review pre 1.0
Date: Sun, 25 Jul 2004 20:13:46 +0100
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
X-Priority: 3
X-MSMail-Priority: Normal
X-Mailer: Microsoft Outlook Express 5.50.4133.2400
X-MimeOLE: Produced By Microsoft MimeOLE V5.50.4133.2400
X-Virus-Checked: Checked
X-Spam-Rating: minotaur-2.apache.org 1.6.2 0/1000/N

Congratulations on the work put into [math]!

Stephen


----- Original Message -----
From: "Phil Steitz" <phil@steitz.com>
To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
Sent: Sunday, July 25, 2004 7:13 PM
Subject: Re: [math] Design review pre 1.0


> Updated response interspersed.  Pls correct / comment as necessary...
>
> Stephen Colebourne wrote:
> > I have performed a lightweight review of [math] from a code/style POV
(not
> > technically as I'm not mathematical...)
> >
> > 1) Remember your public API that you must maintain includes both public
and
> > protected classes/methods/fields. Are you confident that every
method/field
> > (especially fields) are correctly and suitably named for you to want to
> > keep?
>
> Let's just say we have done our best ;-)
> >
> > 2) Wherever there is implements Serializable you should have a
> > serialVersionUID static field. And are you confident that the
> > implementations are well enough established to allow for serialization?
What
> > about transient fields? Serialization is definitely not about adding the
> > implements clause to everything.
>
> All serializable classes have serialVersionUIDs. Some serialization
> incompatible changes may be introduced in .x releases (prior to 2.0); but
> the current impls work correctly and have working tests.
> >
> > 3) Javadocs are sometimes thin. On occasions, they are written as
paragraphs
> > visually but without the HTML <p> tag. (eg. UnivariateRealSolver) or
> > missing, eg StandardDeviation
>
> Javadocs have been improved and formatting fixed.  Class javadoc may be
> thin for classes that just implement a simple interface (e.g. some of the
> distribution classes); but method javadoc is complete.  User Guide is also
> now complete.
> >
> > 4) You might want to consider separating interfaces from
implementations.
> > Perhaps all interfaces in the base package, or impl subpackages. Then
again
> > the current layout may be best.
>
> Decided not to separate within packages.
> >
> > 5) Is double suitable for these calculations? Should the strictfp flag
be
> > used? (I have no idea as to the answer, but I have to ask)
>
> Decided to wait on adding strictfp support to later release, as if we do
> this, users should be able to choose, since performance impact can be
> significant.
> >
> > 6) ComplexFormat doesn't extend the JDK Format class
>
> Fixed.
> >
> > 7) ComplexMath is a utils class, which would be named ComplexMathUtils
in
> > lang or collections (although the JDK Math class sets an alternate
> > precedent)
>
> Fixed
>
> The constructor is private, which I would recommend is changed to
> > public (aids tools like Velocity). Also Beta/Erf/Gamma.
>
> Not changed (no consensus to change).
> >
> > 8) @author tags are often missing
>
> Not changed (consensus is no author tags).  If "anonymous" tags are j-c
> standard, need to write a script to add these.
> >
> > 9) Factorys use the method newInstance() to obtain an instance -
> > getInstance() seems more typical of commons, and allows you to return a
> > cached value.
>
> Factories are dynamically configurable, so consensus was to let the client
> handle caching factory instances (stay with newInstance() semantics).
> Internal clients have been changed to cache (default) factory instances.
> >
> > 10) SummaryStatistics/DescriptiveStatistics are factories but aren't
named
> > as such.
>
> Consensus was that these are not really factories, so names have not been
> changed.
>
>   They also use Class.forName() which is dubious in a class loader
> > environment.
>
> Fixed.
> >
> > 11) Ensure there are no TODO comments in the Javadoc (OK in code) For
> > example GammaDistributionImpl
>
> Fixed
> >
> > 12) Should EmpiricalDistribution really have methods taking File and
String
> > filePath - is File not enough? The implementation actually seems to do
> > something different (beware file encodings)
> >
> Fixed.
>
> > 13) Ensure every class has a constructor - never rely on the auto
generated
> > one, for example BivariateRegression
>
> Fixed.
> >
> > 14) Some import statements are rather screwed visually, eg Product
> >
> Fixed.
>
> > 15) Dependencies!!!!
>
> No (hard) runtime dependencies left. [discovery] and [logging] are
> required to compile [math] and to use the factory configuration features
> that they support. Factory methods that use [discovery] have been modified
> to catch NoClassDefFound and return instances of default implementations.
>
> Phil
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>


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