Skip to content

SetProperty and ListProperty don't properly model concrete collection types #443

@ljacqu

Description

@ljacqu

As seen in the PR for #383, extending SetProperty for EnumSetPropertyType has problems:

    public EnumSetProperty(String path, Class<E> enumClass, EnumSet<E> defaultValue) {
        // Compile error, EnumSetProperty must be unchecked
        super(new EnumSetPropertyType<>(enumClass), path, defaultValue);
    }

This is because:

  • SetProperty<E> extends TypeBasedProperty<Set<E>>
  • EnumSetPropertyType<E extends Enum<E>> extends CollectionPropertyType<E, EnumSet<E>>
    So the Property wrapper for EnumSetPropertyType should be a Property<EnumSet<E>>, not Property<Set<E>>

How to deal with this?

  • I like a general parent for lists and sets (respectively), so that if we need to deal with Property classes, we can do instanceof on general sounding types without needing to list all implementations separately. Hierarchy-wise, it looks good for an enum set property to be of type set property.
  • We could make SetProperty more generic to be something like SetProperty<E, S extends Set<E>> extends Property<S> but that might be clunky for the general use case? Especially needing to then declare set property constants as SetProperty<String, Set<String>> everywhere...
    • Introduce an extension??
    • Introduce an abstract collection property that SetProperty and EnumSetProperty can extend? EnumSetProperty would no longer be a SetProperty, but that might technically be true: SetPropertyType and EnumSetPropertyType are also not parent-child, but siblings
  • Ignore this discrepancy as I think right now it has no impact so long as no PropertyType really strictly assumes only its implementation of a set (or similar) can come in.

Metadata

Metadata

Assignees

No one assigned

    Labels

    architectureDifficult architectural questions

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions