On Sun, Aug 12, 2001 at 09:54:37PM +1000, Peter Donald wrote:
> On Sun, 12 Aug 2001 21:06, Jeff Turner wrote:
> > A question, mainly for Pete,
> >
> > Enum and ValuedEnum both have public constructors:
> >
> > public Enum( final String name );
> > public Enum( final String name, final Map map );
> > public ValuedEnum( final String name, final int value );
> > public ValuedEnum( final String name, final int value, final Map map );
> >
> > Because they're public, any class can add more Enum items at any time,
> > so the Enum is not type-safe at all. Possibly a security hazard if
> > anyone was relying on subclasses' type-safety.
> >
> > Since the only time they should ever be called is within a subclass,
> > would it be better to make them protected? This technically breaks
> > backwards-compatibility, but only where the class is being incorrectly
> > used.
>
> It used to be protected at one stage ... can't seem to recall why it was made
> public. Is there any code in Avalon that it would break?
Er.. yes, actually.
Two cases in Phoenix:
src/java/org/apache/avalon/framework/camelot/State.java
src/java/org/apache/avalon/phoenix/components/phases/BlockDAG.java
In both cases, moving the constructor calls from outside to inside is
pretty easy, and the code looks just as good. I don't think these
examples represent a new use-case for non-typesafe Enums.
Still, it's a backwards-incompatible change. Should we create a CVS
branch "avalon-5_0" for things like this?
Diffs for the avalon and phoenix changes attached.
--Jeff
> Cheers,
>
> Pete
>
> *-----------------------------------------------------*
> | "Faced with the choice between changing one's mind, |
> | and proving that there is no need to do so - almost |
> | everyone gets busy on the proof." |
> | - John Kenneth Galbraith |
> *-----------------------------------------------------*
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
Index: src/java/org/apache/avalon/framework/Enum.java
===================================================================
RCS file:
/home/cvs/jakarta-avalon/src/java/org/apache/avalon/framework/Enum.java,v
retrieving revision 1.3
diff -u -r1.3 Enum.java
--- src/java/org/apache/avalon/framework/Enum.java 2001/08/12 10:48:01
1.3
+++ src/java/org/apache/avalon/framework/Enum.java 2001/08/12 13:05:31
@@ -60,7 +60,7 @@
* Constructor to add a new named item.
* @param name Name of the item.
*/
- public Enum( final String name )
+ protected Enum( final String name )
{
this( name, null );
}
@@ -71,7 +71,7 @@
* @param map A <code>Map</code>, to which will be added a pointer to the
newly constructed
* object.
*/
- public Enum( final String name, final Map map )
+ protected Enum( final String name, final Map map )
{
m_name = name;
if( null != map )
Index: src/java/org/apache/avalon/framework/ValuedEnum.java
===================================================================
RCS file:
/home/cvs/jakarta-avalon/src/java/org/apache/avalon/framework/ValuedEnum.java,v
retrieving revision 1.5
diff -u -r1.5 ValuedEnum.java
--- src/java/org/apache/avalon/framework/ValuedEnum.java 2001/08/12
10:48:01 1.5
+++ src/java/org/apache/avalon/framework/ValuedEnum.java 2001/08/12
13:05:31
@@ -69,7 +69,7 @@
* @param name the name of enum item.
* @param value the value of enum item.
*/
- public ValuedEnum( final String name, final int value )
+ protected ValuedEnum( final String name, final int value )
{
this( name, value, null );
}
@@ -82,7 +82,7 @@
* @param value the value of enum item.
* @param map the <code>Map</code> to add enum item to.
*/
- public ValuedEnum( final String name, final int value, final Map map )
+ protected ValuedEnum( final String name, final int value, final Map map )
{
super( name, map );
m_value = value;
Index: src/java/org/apache/avalon/framework/camelot/State.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon-phoenix/src/java/org/apache/avalon/framework/camelot/State.java,v
retrieving revision 1.1
diff -u -r1.1 State.java
--- src/java/org/apache/avalon/framework/camelot/State.java 2001/04/27
05:03:30 1.1
+++ src/java/org/apache/avalon/framework/camelot/State.java 2001/08/12
12:51:31
@@ -14,10 +14,17 @@
*
* @author <a href="mailto:[EMAIL PROTECTED]">Peter Donald</a>
*/
-public class State
+public final class State
extends ValuedEnum
{
- public State( final String name, final int value )
+
+ //A list of constants representing phases in Blocks lifecycle.
+ //Each phase is made up of a number of stages.
+ public final static State BASE = new State( "BASE", 0 );
+ public final static State STARTEDUP = new State( "STARTEDUP", 10 );
+ public final static State SHUTDOWN = new State( "SHUTDOWN", 20 );
+
+ private State( final String name, final int value )
{
super( name, value );
}
Index:
src/java/org/apache/avalon/phoenix/components/application/DefaultServerApplication.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon-phoenix/src/java/org/apache/avalon/phoenix/components/application/DefaultServerApplication.java,v
retrieving revision 1.4
diff -u -r1.4 DefaultServerApplication.java
---
src/java/org/apache/avalon/phoenix/components/application/DefaultServerApplication.java
2001/07/17 07:54:35 1.4
+++
src/java/org/apache/avalon/phoenix/components/application/DefaultServerApplication.java
2001/08/12 12:51:33
@@ -164,13 +164,13 @@
{
PhaseEntry entry = new PhaseEntry();
entry.m_visitor = new StartupPhase();
- entry.m_traversal = BlockDAG.FORWARD;
+ entry.m_traversal = BlockDAG.Traversal.FORWARD;
m_phases.put( "startup", entry );
setupComponent( entry.m_visitor, "StartupPhase" );
entry = new PhaseEntry();
entry.m_visitor = new ShutdownPhase();
- entry.m_traversal = BlockDAG.REVERSE;
+ entry.m_traversal = BlockDAG.Traversal.REVERSE;
m_phases.put( "shutdown", entry );
setupComponent( entry.m_visitor, "ShutdownPhase" );
}
Index: src/java/org/apache/avalon/phoenix/components/kapi/BlockEntry.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon-phoenix/src/java/org/apache/avalon/phoenix/components/kapi/BlockEntry.java,v
retrieving revision 1.1
diff -u -r1.1 BlockEntry.java
--- src/java/org/apache/avalon/phoenix/components/kapi/BlockEntry.java
2001/07/17 07:05:45 1.1
+++ src/java/org/apache/avalon/phoenix/components/kapi/BlockEntry.java
2001/08/12 12:51:34
@@ -21,12 +21,6 @@
public class BlockEntry
extends Entry
{
- //A list of constants representing phases in Blocks lifecycle.
- //Each phase is made up of a number of stages.
- public final static State BASE = new State( "BASE", 0 );
- public final static State STARTEDUP = new State( "STARTEDUP", 10 );
- public final static State SHUTDOWN = new State( "SHUTDOWN", 20 );
-
private final RoleEntry[] m_roleEntrys;
private final String m_name;
@@ -38,7 +32,7 @@
m_name = name;
m_roleEntrys = roleEntrys;
setLocator( locator );
- setState( BASE );
+ setState( State.BASE );
}
public String getName()
Index: src/java/org/apache/avalon/phoenix/components/phases/BlockDAG.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon-phoenix/src/java/org/apache/avalon/phoenix/components/phases/BlockDAG.java,v
retrieving revision 1.3
diff -u -r1.3 BlockDAG.java
--- src/java/org/apache/avalon/phoenix/components/phases/BlockDAG.java
2001/07/17 12:23:51 1.3
+++ src/java/org/apache/avalon/phoenix/components/phases/BlockDAG.java
2001/08/12 12:51:35
@@ -37,13 +37,13 @@
private static final Resources REZ =
ResourceManager.getPackageResources( BlockDAG.class );
- public final static Traversal FORWARD = new Traversal( "FORWARD" );
- public final static Traversal REVERSE = new Traversal( "REVERSE" );
- public final static Traversal LINEAR = new Traversal( "LINEAR" );
-
public final static class Traversal
extends Enum
{
+ public final static Traversal FORWARD = new Traversal(
"FORWARD" );
+ public final static Traversal REVERSE = new Traversal(
"REVERSE" );
+ public final static Traversal LINEAR = new Traversal( "LINEAR" );
+
private Traversal( final String name )
{
super( name );
@@ -116,7 +116,7 @@
final RoleEntry roleEntry = entry.getRoleEntry( role );
final String dependencyName = roleEntry.getName();
final BlockEntry dependency = getBlockEntry( dependencyName );
- visitBlock( dependencyName, dependency, visitor, FORWARD,
completed );
+ visitBlock( dependencyName, dependency, visitor,
Traversal.FORWARD, completed );
}
}
@@ -162,7 +162,7 @@
}
//finally try to traverse block
- visitBlock( blockName, entry, visitor, REVERSE, completed
);
+ visitBlock( blockName, entry, visitor, Traversal.REVERSE,
completed );
}
}
}
@@ -179,11 +179,11 @@
if( completed.contains( name ) ) return;
completed.add( name );
- if( FORWARD == traversal )
+ if( Traversal.FORWARD == traversal )
{
visitDependencies( name, entry, visitor, completed );
}
- else if( REVERSE == traversal )
+ else if( Traversal.REVERSE == traversal )
{
visitReverseDependencies( name, visitor, completed );
}
Index: src/java/org/apache/avalon/phoenix/components/phases/ShutdownPhase.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon-phoenix/src/java/org/apache/avalon/phoenix/components/phases/ShutdownPhase.java,v
retrieving revision 1.3
diff -u -r1.3 ShutdownPhase.java
--- src/java/org/apache/avalon/phoenix/components/phases/ShutdownPhase.java
2001/07/17 12:23:51 1.3
+++ src/java/org/apache/avalon/phoenix/components/phases/ShutdownPhase.java
2001/08/12 12:51:36
@@ -11,6 +11,7 @@
import org.apache.avalon.framework.activity.Disposable;
import org.apache.avalon.framework.activity.Startable;
import org.apache.avalon.framework.camelot.Container;
+import org.apache.avalon.framework.camelot.State;
import org.apache.avalon.framework.component.ComponentException;
import org.apache.avalon.framework.component.ComponentManager;
import org.apache.avalon.framework.component.Composable;
@@ -51,7 +52,7 @@
public void visitBlock( final String name, final BlockEntry entry )
throws Exception
{
- if( entry.getState() != BlockEntry.STARTEDUP ) return;
+ if( entry.getState() != State.STARTEDUP ) return;
if( getLogger().isInfoEnabled() )
{
@@ -111,7 +112,7 @@
//Destruction stage
getLogger().debug( REZ.getString( "shutdown.notice.destroy.pre" ) );
entry.setInstance( null );
- entry.setState( BlockEntry.SHUTDOWN );
+ entry.setState( State.SHUTDOWN );
getLogger().debug( REZ.getString( "shutdown.notice.destroy.success" )
);
try
Index: src/java/org/apache/avalon/phoenix/components/phases/StartupPhase.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon-phoenix/src/java/org/apache/avalon/phoenix/components/phases/StartupPhase.java,v
retrieving revision 1.4
diff -u -r1.4 StartupPhase.java
--- src/java/org/apache/avalon/phoenix/components/phases/StartupPhase.java
2001/07/17 13:24:00 1.4
+++ src/java/org/apache/avalon/phoenix/components/phases/StartupPhase.java
2001/08/12 12:51:38
@@ -13,6 +13,7 @@
import org.apache.avalon.framework.CascadingException;
import org.apache.avalon.framework.activity.Initializable;
import org.apache.avalon.framework.activity.Startable;
+import org.apache.avalon.framework.camelot.State;
import org.apache.avalon.framework.camelot.Container;
import org.apache.avalon.framework.camelot.ContainerException;
import org.apache.avalon.framework.camelot.Entry;
@@ -98,7 +99,7 @@
public void visitBlock( final String name, final BlockEntry entry )
throws Exception
{
- if( entry.getState() != BlockEntry.BASE &&
+ if( entry.getState() != State.BASE &&
null != entry.getState() ) return;
if( getLogger().isInfoEnabled() )
@@ -181,7 +182,7 @@
getLogger().debug( REZ.getString(
"startup.notice.start.success" ) );
}
- entry.setState( BlockEntry.STARTEDUP );
+ entry.setState( State.STARTEDUP );
}
catch( final Exception e )
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]