raml-org / raml-spec

RAML Specification
http://raml.org
3.87k stars 858 forks source link

multiple inheritance code generation boilerplate #272

Open fededonna opened 8 years ago

fededonna commented 8 years ago

@usarid and @petrochenko-pavel-a I think we should review the multiple inheritance for type system.

The following example shows the case

#%RAML 1.0
title: multiInheritance
types:
  Searchable:
    type: object
    properties:
      index:
        type: integer
        minimum: 1
        maximum: 100
      [a-zA-Z]:
        type: string
        displayName: searchTerm
  Bookmarkable:
    type: object
    properties:
      bookmark: Bookmark []
  Bookmark:
    type: object
    properties:
      page:
        type: integer
        minimum: 1
      line:
        type: integer
        minimum: 1
      [a-zA-z]:
        type: string
        displayName: bookmarkName
  Page:
    type: object
    properties:
      bookmarks: Bookmark[]
  Book:
    type: [Bookmarkable, Searchable]
    properties:
      page:
        type: Page[]
        minItems: 1
      references?: Book[]

boilerplate

The issue here, as I see it, is that we can model interfaces with types and we will create orphan classes that are part of the model and will never be used.

I don't see a real case where multiple inheritance adds value to the final api that can't be model without having multiple inheritance.

machaval commented 8 years ago

One question are we still targeting Xml, if so multiple inheritance is going to be really complicated to follow.

On Fri, Mar 4, 2016 at 11:55 AM, Federico Donnarumma < notifications@github.com> wrote:

@usarid https://github.com/usarid and @petrochenko-pavel-a https://github.com/petrochenko-pavel-a I think we should review the multiple inheritance for type system.

The following example shows the case

%RAML 1.0title: multiInheritancetypes:

Searchable: type: object properties: index: type: integer minimum: 1 maximum: 100 [a-zA-Z]: type: string displayName: searchTerm Bookmarkable: type: object properties: bookmark: Bookmark [] Bookmark: type: object properties: page: type: integer minimum: 1 line: type: integer minimum: 1 [a-zA-z]: type: string displayName: bookmarkName Page: type: object properties: bookmarks: Bookmark[] Book: type: [Bookmarkable, Searchable] properties: page: type: Page[] minItems: 1 references?: Book[]

[image: boilerplate] https://cloud.githubusercontent.com/assets/1690404/13530165/c6683452-e1fe-11e5-8917-854ab5f0e5a6.png

The issue here, as I see it, is that we can model interfaces with types and we will create orphan classes that are part of the model and will never be used.

I don't see a real case where multiple inheritance adds value to the final api that can't be model without having multiple inheritance.

— Reply to this email directly or view it on GitHub https://github.com/raml-org/raml-spec/issues/272.

petrochenko-pavel-a commented 8 years ago

@fededonna

I don't see a real case where multiple inheritance adds value to the final api that can't be model without >having multiple inheritance.

I can not say that I am a big fun of multiple inheritance. (I am Java developer). But I see it is as a pretty usable when composing things from small reusable pieces (JSON schema)(Of course you can argue that this is bad design, but not OOP people still like to use it).

Another reason is that a lot of languages supports this in some manner: https://en.wikipedia.org/wiki/Multiple_inheritance https://en.wikipedia.org/wiki/Trait_(computer_programming)#Supported_languages

So as for me this is a nice to have feature which is important when mapping types from this languages to RAML

@machaval

going to be really complicated to follow.

You always may transform type hierarchy to make it suitable to target type system. Imaging mapping to language which only have struct keyword

petrochenko-pavel-a commented 8 years ago

Another thing is that in fact code generator is free to inherit Book class from Bookmark or Searchable classes

fededonna commented 8 years ago

@petrochenko-pavel-a

So as for me this is a nice to have feature which is important when mapping types from this languages to RAML

can you show a real example of any API which takes advantage of multiple inheritance in a type and can not be accomplished with composition over inheritance?

sichvoge commented 8 years ago

I can see some use cases where that might be useful like query parameters, but wonder if these use cases only take 10% of the majority and everything else don't need multiple inheritance. The main source format here would be XML and JSON, but there are language generation processes and not all languages support it for a reason.

I am not 100% convinced on supporting it. M2C

That does not mean someone can convince us here ;)

machaval commented 8 years ago
  1. In most of the languages that are here Scala , Java multiple inheritance is only allowed for behavior and not state. That is exactly the opposite of what we are doing.
  2. My question is Raml is target for machines or humans?. Do I need to have a Compiler with 1000 rules in order to understand what XML or JSON I need to send the api something is not good. This is exactly why people thought Rest and Json was better than WSDL. WSDL is very hard to read and was design as a first goal for mapping OOL to a type system like XML.

On Fri, Mar 4, 2016 at 12:16 PM, petrochenko-pavel-a < notifications@github.com> wrote:

Another thing is that in fact code generator is free to inherit Book class from Bookmark or Searchable classes

— Reply to this email directly or view it on GitHub https://github.com/raml-org/raml-spec/issues/272#issuecomment-192317889.

petrochenko-pavel-a commented 8 years ago
  1. My question is Raml is target for machines or humans?.

May you please describe your point in a bit more details.

  1. In most of the languages that are here Scala , Java multiple inheritance.

If talk about inheritance for reusability than I think most of languages actually supports it : mixins - Scala, Multiple inheritance (C++, Python), Traits - PHP, ...

As for me our RAML types does not have a behaviour, so I threat them as interfaces describing structure/API of data.

fededonna commented 8 years ago

As for me our RAML types does not have a behaviour, so I threat them as interfaces describing structure/API of data.

So you need interfaces here to describe structure? I think that that's not the intention of interfaces at all.

petrochenko-pavel-a commented 8 years ago

Intention of interfaces is to describe a contract between boundaries of computing system ( https://en.wikipedia.org/wiki/Interface_(computing)) as you may see from the article despite of common meaning of interface in OOP languages they can be used to describe contracts on the data to

fededonna commented 8 years ago

from your link:

In object-oriented languages, the term interface is often used to define an abstract type that contains no data or code, but defines behaviors as method signatures. A class having code and data for all the methods corresponding to that interface is said to implement that interface.[4] Furthermore, a class can implement multiple interfaces, and hence can be of different types at the same time.[5]

It's talking about behaviour not data structure.

petrochenko-pavel-a commented 8 years ago

Interfaces between software components can provide: constants, data types, types of procedures, exception specifications and method signatures. Sometimes, public variables are also defined as part of an interface.

As I wrote earlier representing contract on behaviour is an often case, but it is not only one, the real thing is that it is a contract definition between two components

fededonna commented 8 years ago

You will create concrete objects with no use in your model. You can not decide if it's an class or an interface analyzing what the user expected to do with that particular type in the context of a parser. Please provide a concrete example where that multiple inheritance definition enrich your api.

petrochenko-pavel-a commented 8 years ago

As you may see from this example:

#%RAML 1.0 Api
mediaType: application/json
title: Api
types:
  Sortable:
    properties:
      sortCriteria: string
      isAscending: boolean
  Pageable:
    properties:
      pageNumber: number
      totalPages: number
      pageSize: number
  Filter:
    properties:
      property: string
      operator:
        type: string
        enum: [EQUAL,NOT_EQUAL, GREATER, LESS]
  Searchable:
    properties:
      appliedFilters: Filter[]
  FullTextQuery:
    properties:
      textQuery: string
  Device: object
  User: object
  Page: object
/devices:
  get:
    responses:
      200:
        body:
          type: [Sortable, Searchable]
          properties:
            items: Device[]
/users:
  get:
    responses:
      200:
        body:
          type: [Sortable]
          properties:
            items: User[]
/pages:
  get:
    responses:
      200:
        body:
          type: [Sortable,FullTextQuery,Pageable]
          properties:
             items: Page[]

As you may see from this example, multiple inheritance is pretty handy when composing an object structure definition from a reusable pieces and allows to avoid unnecessary duplication of property definitions.

Of course someone can argue that if you design your type hierarchy properly there is no need for composing an objects from a multiple pieces, and good OOD courses teach you how to compose architecture without need to use multiple inheritance which is potentially dangerous and may introduce conflicts, but it is counter arguable by the fact that RAML types should be simple for a not technical user who just wants to describe his data. In fact it is pretty similar to traits (for which we already have multiple inheritance in RAML)

usarid commented 8 years ago

@petrochenko-pavel-a already said this but I thought it's important enough to repeat:

[ @machaval ] In most of the languages that are here Scala , Java multiple inheritance is only allowed for behavior and not state. That is exactly the opposite of what we are doing. An API call is just as analogous to a function call as anything else, and the data passed in that call may well be treated as parameters of that function as well as stateful data. Pure REST, of course, would lean towards regarding the entity as state, but that's just one way people use API calls. And since we're all familiar with using multiple interfaces to mix together interface behaviors, that's pretty much what's going on with multiple inheritance here too, in the context of APIs -- i.e. interfaces.

Will these data types be used to describe stateful entities? Yes they will, and in fact that's what we're running into here in this discussion. When generating clients and servers, the obvious behavior is to look at the body (payload) and turn it into a set of (stateful) entities. You're trying to translate from a behavioral, function-invoking, interface-like context to a structural, stateful, class-like context. That certainly would be smoother in general if the original context were also class-like.

But it's not. For the non-body part of the API, specifically for query parameters, it's very convenient to compose via type extensions. But also for the body part of the API, it's very convenient to compose pieces of interfaces for data you need to receive in your API, that should not have been included in some base type:

types:
  product:
    properties:
      name: string
      price: number
  shipper:
    properties:
      name:
        enum: [ FedEx, UPS, USPS, drone ]
  interestMetrics:
    properties:
      timeSpentLooking: number
      activityLevel: integer
  cartAdditions:
    properties:
      stuff:
        type: array
        items: 
          type: product
      preferredShipper:
        type: shipper
      interestingStuff:
        type: array
        items: 
          type: [ product, interestMetrics ]
      interestingShippers:
        type: array
        items: 
          type: [ shipper, interestMetrics ]
/cart:
  post:
    body: 
      type: cartAdditions

Of course I could \ have modeled the above as single inheritance, by defining intermediate types to carry the extra properties, e.g. interestingProduct that inherits from product and adds the timeSpentLooking and activityLevel properties, but I'd lose the concept of the interestMetrics type to a bunch of copy/paste. Alternatively, I could have made composition explicit, e.g.

interestingProduct:
  properties:
    product:
      type: product
    interestMetrics:
      type: interestMetrics
interestingShipper:
  properties:
    product:
      type: shipper
    interestMetrics:
      type: interestMetrics

And that's pretty much what the code generator needs to do: recreate these intermediate classes if the original type design didn't have them. But for the type designer, there's a bunch of overhead in the explicit composition, not just in the type design but also in the data to be shipped. Here's an example with the original type design (I'll show it in YAML but you can feel free to see it in JSON too via http://yamltojson.com/):

stuff:
- name: battery
  price: 3.99
- name: ribbon
  price: 1.74
preferredShipper:
  name: FedEx
interestingStuff:
  - name: propeller
    price: 6.98
    timeSpentLooking: 12.8
    activityLevel: 5
  - name: box
    price: 0.85
    timeSpentLooking: 4.5
    activityLevel: 2
interestingShippers:
  - name: drone
    timeSpentLooking: 46.5
    activityLevel: 5

Here's the example if you made the composition more explicit:

stuff:
- name: battery
  price: 3.99
- name: ribbon
  price: 1.74
preferredShipper:
  name: FedEx
interestingStuff:
  - product:
      name: propeller
      price: 6.98
    interestMetrics:
      timeSpentLooking: 12.8
      activityLevel: 5
  - product:
      name: box
      price: 0.85
    interestMetrics:
      timeSpentLooking: 4.5
      activityLevel: 2
interestingShippers:
  - shipper:
      name: drone
    interestMetrics:
      timeSpentLooking: 46.5
      activityLevel: 5

So should users be forced to model their data in this second way?

Also, note that this sort of explicit composition doesn't work at all for query parameters, where you only get one level of properties, not nesting. (Technically you can nest but few people choose to.)

fededonna commented 8 years ago

@usarid can you clarify a little bit this part?

      interestingStuff:
        type: array
        items: 
          type: [ product, interestMetrics ]

What is the final goal of this definition. Because with this you won´t accept any product if it's not interesting. And also you are duplicating the data because

      stuff:
        type: array
        items: 
          type: product

Already has the product in it so you could model "interestingProduct" as a tuple of interesMetrics and a reference to the product already added to the cart and not the exact same data that's already in product because that would make you API redundant.

And how would you model the back-end implementation of this?

usarid commented 8 years ago

@fededonna take a look at the example to see there is no issue with duplication. The API is taking an array of products the user wants to buy (battery, ribbon), as well as an array of products the user was interested in (propeller, box) -- and for those, each comes with information about the level of interest. Yes, that information happens to be required in this example

As for the back end: that's up to the implementer of the service. For example, they could strip off the interestMetrics parts of the data to feed an analytics system, and use a single class to model any product.

fededonna commented 8 years ago

@usarid I see the thing there is that multiple inheritance is not adding something that composition can't bring(as you've mentioned). It's just a matter of thinking things different. Multiple inheritance is been in OOP since a while and not much languages have followed that path. Maybe we should follow the path of people who's been writing languages for a long time and have in mind that behind this API definition there is a language that maybe doesn't support multiple inheritance and favors composition. Also this is going to cause a discrepancy between his API definition and his real model implementation.

usarid commented 8 years ago

cause a discrepancy between his API definition and his real model implementation I like that, actually: I don't want to encourage a tight coupling between the API and the implementation. If the designer of the API wants to create a surface area that models classes with single inheritance, he can do that easily enough, but he's not forced to. And he's free to relate whatever API surface area makes sense for his users to whatever implementation makes sense for him behind the scenes.

We're not targeting the creation of an OOP language, we're primarily targeting the description of data to be exchanged between systems, and sometimes also data within those systems where the implementer of that system can choose how to do inheritance.

Here's an example that uses type combinations to specify data coming via query parameters:

#%RAML 1.0
title: Test query string options
types:
  paging:
    properties:
      start?:
        type: integer
        default: 0
      pageSize?:
        type: integer
        default: 10
  lat-long:
    properties:
      lat: number
      long: number
  location:
    properties:
      loc: string
/devices:
  get:
    queryString:
      type: [ paging, lat-long | location ]
      examples:
        general:
          content:
            start: 2
            pageSize: 20
            lat: 20.3450
            long: -12.4572
        specific:
          content:
            pageSize: 15
            location: Chicago, IL
        minimal:
          content:
            location: here