[Lexi-developer] Problems with the Registry class

[Stephen C. <cr...@ds...>]

Hi Mark,

In reformatting the class, I've come to the conclusion that there are a 
number of things about it
that are "broken".  I'd like your opinion on my diagnosis / suggestions.

The Registry class is the basis for Lexi's user preferences / properties 
support.  [In fact, it looks
like the original author had significantly bigger plans.]  The model for 
the Registry is as follows:

  *  A Registry contains properties that are triples consisting of a 
name, a value and a type.

  *  Property types consist of Java primitive types, String and Object, 
and arrays of the previous.

  *  Properties are organised as groups so that a Registry property is 
denoted by the property name and
     the group name.

The Registry class seems to be designed  to allow multiple 
implementations of the abstraction, except
that this has been done wrongly IMO.  The javadoc says that you should 
use static "loadXXX" methods
to instantiate a Registry.  These methods create instances of the 
Registry.Impl inner class, which
is a subclass of Registry.  [A bit unusual ...]  The problem is that:

  1)  the Registry class is non-abstract and has a protected 
constructor, and

  2)  the non-static methods of the Registry class are (mostly) 
skeletons which don't do what
      the javadoc says they should do.

So Registry is not a safe class.  If a client creates an instance of 
Registry via the constructor, it gets
a non-functional object.  And it is dangerous as a superclass for a real 
implementation (like the Impl class)
because it contributes disfunctional method implementation.

IMO, the fix is to turn Registry into an abstract class, and make the 
Impl class a non-nested class with
package private access.

Another problem is that it is (IMO) sloppy about the typing of properties:

  1)  Nothing stops an application setting a property with (say) a 
String value when it was previously
      (say) an array of ints.

  2)  If an application tries to get a property with the wrong type, it 
will get a default value.

  3)  If the application uses the referenceGroup, it gets the properties 
in a group as a Properties
      object with the type encoded in the property values.  If the 
Properties object is updated (as
      the PropertySheetDialog class does!), the updates are reflected in 
the registry ... without
      any checking that the registry type prefix makes sense.

IMO, the fix to these problems is to rework the getters and setters, and 
to get rid of the referenceGroup
method.  The getters and setters need to throw an (unchecked I think) 
exception if the application gets
the types wrong.  And we need a better way of setting initial values 
than the current kludge of using
the default value supplied in a getter as an initial value whan the 
property is missing.

What do you reckon?

-- Steve