Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A 'MetaBean' which is generic in the bean's type #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

A 'MetaBean' which is generic in the bean's type #11

wants to merge 3 commits into from

Conversation

nipafx
Copy link
Collaborator

@nipafx nipafx commented Jan 7, 2015

Experimental Branch

Following the idea laid out in #6 this branch experiments with the possibility to make MetaBean generic in the bean's type B.

This entails the following type changes:

  • Bean ~> Bean<B>
  • MetaBean ~> MetaBean<B>
  • MetaProperty<P> ~> MetaPropert<B, P>

As discussed before (see #6 for references), not all methods can be generified. The current state is very smooth, though. All return types are generic and concrete (i.e. not ?) and so are most arguments. The (quite regular) exceptions are:

  • MetaProperty<B, P>:
    • get(Object) could be get(B)
    • set(Object, Object) could be set(B, P)
  • BeanBuilder<B>:
    • get((MetaProperty<?, ?>) could be get((MetaProperty<B, ?>)
    • set((MetaProperty<?, ?>, Object) could be set((MetaProperty<B, ?>, P)

The other changes to implementations and tests follow from this.

nicolaiparlog added 2 commits January 7, 2015 17:22
'MetaBean' is now 'MetaBean<B>' and 'MetaProperty<P>' is 'MetaPropert<B, P>', where 'B' is the type of the bean associated with the meta-bean.
The comments now correctly reflect the generification of 'MetaProperty<P>' to 'MetaProperty<B, P>'.
@@ -103,7 +105,7 @@ static MetaBean of(Class<?> cls) {
*
* @return the stream of properties on the bean, not null
*/
Stream<MetaProperty<?>> metaProperties();
Stream<MetaProperty<B, ?>> metaProperties();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where it starts to get complicated - object hierarchies. An Address might define certain meta-properties, and a subclass CompanyAddress might define some more meta-properties. What is the B type of the meta-properties of CompanyAddress? Saying that B must always be CompanyAddress implies creating a meta-property instance for all properties of CompanyAddress and Address. Whereas it would be useful to be able to inherit the parent meta-properties but use them against the child. IIRC, generics are not well setup to deal with this, but it would be interesting to try. This will affect signatures on MetaProperty as well as here.

@nipafx
Copy link
Collaborator Author

nipafx commented Jan 7, 2015

I wondered about the same thing while implementing this. I decided to pick the easy way out and discuss it here.

This seems to be the root for the problems with the missing wildcards:

Whereas it would be useful to be able to inherit the parent meta-properties but use them against the child.

From what little I can judge, I'd say that this would increase the complexity of the API considerably. First, it would make many occurrences of generics complicated (which is part of your point, I guess). But allowing this would also introduce ambiguity regarding the bean which owns a set of meta-properties. So even if it is clear that some properties come from the same bean it is not trivial to identify from which one. This makes MetaProperty.metaBean() useless in few/some/many/most (?) cases.

E.g.:

public static void main(String[] args) {
    MetaBean bean = null;
    MetaProperty<?> propA = bean.metaProperty("A").get();
    MetaProperty<?> propB = bean.metaProperty("B").get();
    createBeanWithProperties(propA, propB);
}

public static Object createBeanWithProperties(
        MetaProperty<?> propA, MetaProperty<?> propB) {
    BeanBuilder<?> builderForBeanWithPropA = propA.metaBean().beanBuilder();
    BeanBuilder<?> builderForBeanWithPropB = propB.metaBean().beanBuilder();
    // this could be different builders; which one is the correct one?
}

More problems might be hidden under the rock of contravariant properties (e.g. CompanyAddress accepts objects in setStreet whereas Address only accepted strings - or an equivalent example which actually makes some sense). It is also not clear to me whether the meta meta-properties like isBuildable are always valid for both the super- and the subclass.

I wonder about the benefits, though. It would obviously spare some instances. In big systems with deep inheritance hierarchies this might amount to a measurable performance gain. In a more strict setting (i.e. without inheriting meta-properties), this could be approximated if a class's meta-properties would wrap those meta-properties already created for the superclass. Without having tried it, I'd say that this is generally possible with a simple wrapper class.

Another benefit could be that it makes implementing bean builders, bean introspectors and other things easier but I don't see enough of the big picture to have an educated opinion on that.

@jodastephen
Copy link
Owner

So, in Joda-Beans, the superclass meta-properties are inherited by the subclass. This reduces the code generation but does complicate some inheritance aspects. I hadn't really thought about not inheriting, but it might be possible. You make some good points about the definition of the getter/setter changing between superclass and subclass.

The metaProperty.metaBean().builder() case has never come up as an issue in Joda-Beans but could do I suppose. The hassle for Joda-Beans is that it doesn't know what the superclass properties are, as it only processes the source code of the subclass (and cannot see or query the superclass).

It would be good to see an implementation that wraps superclass met-properties for the subclass and how bad/complex that would be. Probably not too bad. My current view is that you are right and Joda-Beans is wrong, although Joda-Beans would be hard to fix, and while more correct, doing the right thing might restrict freedom of implementation of the meta-model for some.

Proof-of-concept implementation of a meta-property A wrapping another meta-property B, where the bean to which B belongs is a superclass of the bean to which A belongs.
@nipafx
Copy link
Collaborator Author

nipafx commented Jan 12, 2015

The hassle for Joda-Beans is that it doesn't know what the superclass properties are, as it only processes the source code of the subclass (and cannot see or query the superclass).

I can not judge why this is so and whether it would be the same for this project. Sounds like an important detail, though.

It would be good to see an implementation that wraps superclass met-properties for the subclass and how bad/complex that would be.

I just committed a proof-of-concept (56fdb26), which I implemented in haste so it might not be spot-on for what we need.

while more correct, doing the right thing might restrict freedom of implementation of the meta-model for some

I guess that's the trade-off: ease of use of the meta-model vs. ease of its construction. Being more strict might make things more complicated for those creating meta-beans but I'm sure it will make it easier to consume it.

I'm curious, though. If I find some time on the weekend, I'll make another branch and try to generify it in a way which would allow inheriting meta-properties across beans.

@nipafx nipafx mentioned this pull request Jan 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants