From: Stephen Crawley <crawley@ds...> - 2005-08-29 13:36:34
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
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?