Hi Andy,
1. Using simple annotations instead of complex attributes
For instance:
@PrimaryKey
instead of:
@Field(primaryKey=true)
(or simply @Id as in JPA).
OK, but the other point of view is where we have a field that needs
several
attributes setting (e.g NullValue, Dependent, PrimaryKey, etc) we end up
with
a long list of annotations against it. In that case a single annotation is
cleaner IMHO. But yes I agree in the case of say one or two attributes to
be
added that it would add more flexibility and be simpler.
This is true.
It doesn't have to be a question of whether we have the attributes in
Field OR
have things like PrimaryKey. We could allow both.
Seems as a good solution. At least @PrimaryKey should be a spearate
annotation.
Other annotations that can be considered:
@Embedded, @Serialized, @Dependent, @Fetch
and maybe less important but still an option:
@Transient, @Required
Also, @Embedded can indicate the combination of:
@Field(embedded="true")
@Element(embedded="true") (after moving from Array and Collection)
@Key(embedded="true") (after moving from Map)
@Value(embedded="true") (after moving from Map)
(anything that is relevant from the above).
i.e. not exactly duplication of @Field(embedded="true"), but rather a
shortcut
for a new more powerful operation that can be equivalent to JPA's @Embedded
and will cover most real uses.
Same for @Serialized and @Dependent, @Index and @Unique
(mainly for collections, but maybe also for maps when keys and values should
share the same setting, or maybe @Serialized and @Dependent should only
cover map values and @Index and @Unique only map keys).
Also, maybe @Index should be defined without unique option, by:
public @interface Indexed { boolean value() default true; }
or maybe even by:
public @interface Unique {}
and simply used with:
@Indexed
Do we really need so many different methods to define a unique index?
Some ideas :-
1. I'd remove "indexed" attribute from @Field since we can have @Index
specified on the field.
2. I'd remove "unique" attribute from @Field since we can have @Unique
specified on the field.
3. I'd change @Index to make "name" and "table" default to "" (so we can
then
just specify @Index against a field (this then means the same as
"indexed=true")
4. I'd change @Unique to make "name" and "table" default to "" (so we can
then
just specify @Unique against a field (this then means the same as
"unique=true")
I noticed now that you already defined @Index and @Unique with:
@Target({ElementType.TYPE, ElementType.FIELD, ElementType.METHOD})
so the steps above (at least steps 3 and 4) are essential.
@Index is defined with a "unique" option for RDBMS convenience IIRC.
2. Discarding @Array, @Collection and @Map
It seems logical to move the attributes of @Array, @Collection and @Map
to
@Element, @Key and @Value. The separation (whose origin is in the XML) is
unclear to me.
Agreed.
The separation in MetaData is JDO1 backwards compatibility (i.e the
"element",
"key", "value" didnt exist in JDO1 and the attributes of their types etc
were
already on the array, collection, map so were left there ... I think).
Obviously with annotations we dont have to consider backwards
compatibility.
3. Merging all annotations to one package
Good separation seems to be very difficult. For instance - @Index, which
is
currently in orm has both columns and fields as attributes. Fields are
not
related to orm - just columns. If the separation remains, at least moving
@Index, @Indices, @Unique, @Uniques, @Element, @Key and @Value that
contain
also non orm information should be pulled out to the main package.
No complaints from me. I had them together at the start and received a
request
to split them. As you say, what to include as "orm" and what not is not
clear. ODBMS usually allow Index, Unique etc whereas these are in the ORM
DTD.
If the issue here is discouraging users from putting "ORM" info as
annotations
then documentation (and annotation tools) is the way to do that.
4. Type attributes should be Class rather than String
For instance, in Field:
Class fieldType();
instead of:
String fieldType() default "";
I think that should be
Class fieldType() void.class;
because otherwise it becomes "required".
Similarly PersistenceCapable.objectIdClass
I guess you are right, just 'default' seems to be missing:
Class fieldType() default void.class;
5. RecursionDepth
Should be added somehow to the fields in @FetchGroup (and removed from
@Field if unused).
I was originally intending on having Field[] as an attribute of
FetchGroup.
Since we only really need the field name and the recursion-depth there it
would be nice to provide notation just for those. What this notation would
be
is not clear to me however.
--
Andy
Maybe we need another annotation @FieldFetch.
IMO reusing fields in fetch groups is also confusing in the XML metadata.
Ilan
