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

Pick up properties from implemented interfaces #22

Closed
Andersmholmgren opened this issue Apr 23, 2016 · 8 comments
Closed

Pick up properties from implemented interfaces #22

Andersmholmgren opened this issue Apr 23, 2016 · 8 comments
Assignees

Comments

@Andersmholmgren
Copy link

Unless I'm missing something you can't currently have a hierarchical model, where you inherit properties from parent models and similarly builders inherit from other builders.

Is this planned?

@dave26199
Copy link

No, it's not planned; the answer is as for @autovalue, which is that there
is simply no way to make operator== work with inheritance:

https://github.com/google/auto/blob/master/value/userguide/howto.md#-have-one-autovalue-class-extend-another

Effective Java goes into some detail, here's an excerpt: "It turns out that
this is a fundamental problem of equivalence relations in object-oriented
languages. There is no way to extend an instantiable class and add a value
component while preserving the equals contract, unless you are willing to
forgo the benefits of object-oriented abstraction."

However, it is possible to implement interfaces, and this is the right
way to treat shared groups of properties. (In fact, it's positively
encouraged -- this is practically a requirement for building powerful
object models).

I believe it should also work to use mixins to share implementation,
although I haven't tried it.

Do you have any use cases in mind that aren't covered by interfaces+mixins?

On Sat, 23 Apr 2016 at 02:34 Andersmholmgren notifications@github.com
wrote:

Unless I'm missing something you can't currently have a hierarchical
model, where you inherit properties from parent models and similarly
builders inherit from other builders.

Is this planned?


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#22

@Andersmholmgren
Copy link
Author

I meant more that the code generator does not pick up inherited properties from implemented parent classes.

If you look at https://github.com/Andersmholmgren/vcore/blob/master/lib/src/model/model.dart#L102
you'll see that I have to duplicate several properties from the parent classes.

Similarly for the builders https://github.com/Andersmholmgren/vcore/blob/master/lib/src/model/model.dart#L118

Other than that it seems to be working for my current use case which is (this project) where I am building a meta model for BuiltValue classes. Similar to OMG's EMOF and Eclipse Modelling Frameworks ECore.

If I get as far as implementing XMI support and mapping ecore to vcore, then in theory we should be able to design our domain models as class diagrams, using eclipse ecore tools then generate BuiltValues + their Builders.
Plus I should then also get things like generating a library for XMLSchema etc largely for free

@Andersmholmgren
Copy link
Author

Forgot to mention a more serious problem.

I was forced to create a custom serialiser (https://github.com/Andersmholmgren/vcore/blob/master/lib/src/model/model.dart#L118) because I have properties using a base type.

abstract class Classifier implements NamedElement {}

abstract class GenericType
    implements Built<GenericType, GenericTypeBuilder>, Classifier {
  Classifier get base;
  BuiltMap<TypeParameter, Classifier> get genericTypeValues;
...
}

Properties like base and genericTypeValues need to use the abstract type Classifier. The Json serialiser generator doesn't cope with this so I needed to create a custom serialiser

Serializers serializers =
    (_$serializers.toBuilder()..add(_classifierSerializer)).build();

//final x = new SerializersBuilder()..;
final _ClassifierSerializer _classifierSerializer = new _ClassifierSerializer();

class _ClassifierSerializer implements StructuredSerializer<Classifier> {
  @override
  Classifier deserialize(Serializers serializers, Iterable serialized,
      {FullType specifiedType: FullType.unspecified}) {
    return serializers.deserialize(serialized);
  }

  @override
  Iterable serialize(Serializers serializers, Classifier object,
      {FullType specifiedType: FullType.unspecified}) {
    return object != null ? serializers.serialize(object) : [];
  }

  @override
  Iterable<Type> get types => [Classifier];

  @override
  String get wireName => 'Classifier';
}

@davidmorgan
Copy link
Collaborator

davidmorgan commented Apr 24, 2016

Good stuff. Let's see, quite a few threads now...

One quick question, why do you define "build" factories, e.g. "factory Property.build"? It should be as nice to use the builder. If you have to define your own factories, the builder isn't doing its job!

Also, it would be a bit neater to use the constructor instead of the builder, so instead of:

    return (new PropertyBuilder()
          ..docComment = docComment
          ..name = name
          ..type = type
          ..isNullable = isNullable ?? false
          ..defaultValue = defaultValue
          ..derivedExpression = derivedExpression)
        .build();

You could do

    return new Property((b) => b
          ..docComment = docComment
          ..name = name
          ..type = type
          ..isNullable = isNullable ?? false
          ..defaultValue = defaultValue
          ..derivedExpression = derivedExpression);

Re: not picking up inherited properties from implemented interfaces, yeah, that should be added. Good point. Could you rename this issue to something more specific for that please? "Pick up properties from implemented interfaces" maybe? Then we can add separate issues for anything else.

Object modelling with built_value is a great idea, that's exactly the sort of thing it's intended for. (Although we never went so formal.)

Re: serializers, yes, this is a planned feature, just not implemented yet. Could you please file an issue over at https://github.com/google/built_json.dart/issues -- something like, "allow non-concrete serialized types provided all implementations are serializable"?

Thanks for all the input! I won't get to work on these this week because I'm on vacation, but hopefully will get to them the week after.

@Andersmholmgren
Copy link
Author

Actually the build ctrs were an experiment. I got the metamodel and code generator to the point where I could generate identical code to what I started with. But I felt the use of builders were making the metamodel a bit verbose so was looking for an alternative.

However, I had missed that I could use the return new Property((b) => b form. I tried that today and it made a big difference, so I regenerated the code without the build ctrs. Thanks for the pointer.

Years ago when I was an architect at an investment bank I got quite into EMF and ecore. We had truly massive domain models (based on the FpML standard) and needed to transform to different related domain models (sometimes bidirectionally).

One of the things that I really liked about EMF is that it gave a real uniformity to certain APIs. You essentially just needed to understand a domain model (e.g. XML Schema). The way you interacted with that model was the same as with any other domain model (FpML, JSON Schema etc).

I'm not sure how far I'll take this but am keen to regain some of the bits I liked from that experience.

I'll rename the issue and create a new one as you suggested

@Andersmholmgren Andersmholmgren changed the title Support for inheritance Pick up properties from implemented interfaces Apr 25, 2016
@davidmorgan
Copy link
Collaborator

Great -- that's how it's intended to be used. Of course, any suggestions, let me know.

I'll try to take a look at these two issues in the next couple of weeks, they should be relatively straightforward. Thanks.

@davidmorgan
Copy link
Collaborator

I had a look into this.

It's straightforward for value types; just count getters from implemented interfaces as properties.

For builders there are a few more cases because of the facility to set a default. If you don't want a default then the generated builder class should have a concrete field. If you do want a default, you need to write the concrete field yourself.

This is doable, but I think I should tidy up the generation code first, adding a data model rather than doing everything "procedurally". That will make this change easier, and hopefully easier to get right!

@Andersmholmgren
Copy link
Author

makes sense. I think it's worth investing in doing it properly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants