brettviren / moo

ruminants on module oriented programming
GNU General Public License v3.0
4 stars 4 forks source link

Object inheritance and ocpp.hpp.j2 declare_record macro #19

Open alessandrothea opened 3 years ago

alessandrothea commented 3 years ago

Hi again,

Playing around I tried to create a C++ struct using the schema inheritance that moo provides, but with no luck. The base class doesn't seem to end up in the generated Struct.hpp file. A look into ocpp.hpp.j2's declare_recordmakes me suspect that the issue that the macro doesn't yet take into account inheritance.

{% macro declare_record(model, t) %}
struct {{t.name}} {
    {% for f in t.fields %}

    // @brief {{f.doc}}
    {{f.item|listify|relpath(model.path)|join("::")}} {{f.name}} = {{cpp.field_default(model.all_types, f)}};
    {% endfor %}
};
{%- endmacro -%}

Is that the case or is there some other trick to make handle inheritance correctly when rendering the C++ structs?

Thanks! Alessandro

P.S.: I meant to check the nlohmann json functions generation, but haven't managed yet.

brettviren commented 3 years ago

Hi @alessandrothea

You've found a big hole.

Inheritance for record types is only supported (if I can even say that much) in schema. Nothing else (templates nor moo.otypes) makes use of it. And, we've yet (until now) developed any schema that tried to use inheritance.

Inheritance doesn't have a conceptual counterpart in JSON Schema so it can not be applied for validation. What does it even mean if one JSON object "inherits" from another? It's an undefined thing with multiple possible interpretations.

I've always been suspicious of even adding "support" for it in moo schema and did so under some slight duress. I'd be happy to remove it.

Any use for inheritance that I can come up in this context can just as well be accomplished by composition. HaveA instead of IsA.

So, if we really must push on whatever "inheritance" might mean in this context, let's come up with some use cases.

Note, there is a sort of inheritance that exists now in that a record is serialized as a "bag of attributes". This was already discussed elsewhere which I can dig up or repeat if needed.

alessandrothea commented 3 years ago

Ah-ha! Ok understood. My purpose was get more familiar with moo and templates so I don't have an immediate answer.

The request for some use cases is very reasonable, but I'm afraid I don't have one ready yet. I think the suggestion for object inheritance came from @glemanmiotto so we should bring her in this conversation, maybe

brettviren commented 3 years ago

Okay. I'll leave this ticket open. If people have input, please give it. And we can also discuss elsewhere.

glehmannmiotto commented 3 years ago

@glehmannmiotto was missing a few characters and thus wasn't notified. ;-) I can think of many use cases in which inheritance is useful. It is true that in several cases one can live with aggregation, but it is not really the same thing. If I describe an application with its general properties (host, executable, environment, ...), it is true that I can describe a specific application as having all the properties of the base application + its own, but logically an application is an application and my specific application should extend from it. A case in which inheritance would be practical is for commands: a generic command is characterised by its identifier, a run control command should extend from it and carry, additionally, the entry and exit state. A specific run control command for a specify application would additionally have custom payloads. And so on. I would say that the use of inheritance is very natural for describing the configuration of a DAQ system and that it is a requirement. So we should find the technical solution that supports it, rather than arguing the other way around. This of course is a long term objective and shouldn't impact what we have in place at present to support the development of the system: I'm not suggesting to throw up everything in the air.

alessandrothea commented 3 years ago

Sorry about that! GitHub is horrible when it comes to find users...

brettviren commented 3 years ago

Hi @glehmannmiotto @alessandrothea

I feel we must think of data structure "inheritance" somewhat differently than we do class inheritance because there is no behavior involved (no methods, no C++ virtual). Instead, it is just one of two orthogonal patterns which may be applied to supply field information to a record type.

The bases pattern most closely matches our notion of C++ struct inheritance. However, the fields pattern along with the moo onljs.hpp.j2 (and newer omsgp.hpp.j2) templates also provide for an inheritance pattern (maybe one qualified with the terms "ad-hoc" or "facet"). The templates produce C++ code that allow any type of C++ struct to be filled from any underlying data object as long as the object attributes provide all the required fields (or, any missing object attributes are covered by default field values in the schema associated with the C++ struct).

If C++ struct inheritance is required in our data structures then I will extend the moo o*.hpp.j2 templates so that they reflect the bases pattern into the generated C++. But, given that there is already an inheritance pattern in moo schema and templates I wanted to push back a bit to see if it was suitable or at least raise awareness about it.

BTW, this deserialization-based inheritance pattern can also be used to remove the need for the any type in some cases. It was only recently that I realized our "command dispatch protocol" does not need any. The use of any brings some C++ issues, which we've worked around, so considering any improvement here is maybe water under the bridge.