Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: <derby-dev@db.apache.org>
Received-SPF: neutral (asf.osuosl.org: local policy)
Message-ID: <4379EF2E.8030103@debrunners.com>
Date: Tue, 15 Nov 2005 06:22:38 -0800
From: Daniel John Debrunner <djd@debrunners.com>
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US;
 rv:1.7.3) Gecko/20040910
MIME-Version: 1.0
To: derby-dev@db.apache.org
Subject: Re: [jira] Updated: (DERBY-289) Enable code sharing between Derby
 client and engine
References: <922737490.1131395000353.JavaMail.jira@ajax.apache.org>
 <437099FA.9000001@debrunners.com> <4371307C.5000608@sun.com>
In-Reply-To: <4371307C.5000608@sun.com>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

David W. Van Couvering wrote:

> Hi, Dan, I get it now.  Yes, I think this is feasible, and I'll work on
> that.  The network client code could do something like
> 
> throw ExceptionUtil.newSQLException(SQLState.ERRCODE, arg1, arg2);
> 
> Meanwhile, your inspection of and comments on the rest of the patch
> would be much appreciated!

I got confused due to the naming of the classes
org.apache.derby.common.CommonFeatures and CommonInfo. Given the concept
of shared code and the use of 'common' of the top-level package and
source directory, I thought they were generic classes. But it turns out
they make up a specific instances of a shared component. If you like a
common shared component, or common common component. :-)

For instance, I started reading the code as if only one shared component
could exist. Maybe I'm still confused, because CommonInfo is a specific
instance of SharedComponentInfo but it uses a constant from
CommonFeatures to implement its getMaxFeatureId(). The comments in
CommonFeatures implies it holds the constants for multiple shared
components, so why is MAX_FEATURE defined there, shouldn't it be
specific to a shared component? And what is the benefit of having the
constants in a single interface, why not have the feature constants in
the shared component class, e.g. CommonInfo in this case? [Writing this
paragraph I also was confused, it seemed like CommonFeatures would have
code and CommonInfo would have the constants].

Looking to the future on this issue, would you expect the code for, say,
a DRDA common component to be in the package org.apache.derby.common, or
something like org.apache.derby.common.drda? The latter seems better,
which would mean the common shared component code would logically move
to org.apache.derby.common.common.

Maybe it would be better to stick to the single terminology of shared
for the general concept, leading to java/shared/org/apache/derby/shared,
and use common for the specific shared component that is common (to?).

The concept of max feature id must remain an implementation detail, not
used by consumers of a shared component, as it won't work in the future
if features are deprecated. The non-public aspect of getMaxFeatureId()
in SharedComponentInfo seems to be the intent, but then to match that,
the MAX_FEATURE constant needs to be not public.

MessageUtil.EN is incorrectly named, and is not required, why not use
java.util.Locale.US?

For the MessageUtil classes, are all the public methods intended to be
part of the public api, including methods like getCompleteMessage,
formatMessage, composeDefaultMessage?

package.html and the comments in the classes are not in sync:

package.html: - Every time you add a
new feature that is not forward-compatible, you need to add a new feature id

CommonFeatures: Every time a new feature is added that a user of a
shared component depends upon, a feature id should be added here.


Dan.