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.
