search

eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

OCLStandardLibraryImpl is neither extensible nor reusable. #350

Closed eclipse-ocl-bot closed 1 month ago

eclipse-ocl-bot commented 1 month ago

| --- | --- | | Bugzilla Link | 258973 | | Status | CLOSED FIXED | | Importance | P3 enhancement | | Reported | Dec 16, 2008 12:14 EDT | | Modified | Aug 28, 2024 02:39 EDT | | Version | 1.2.0 | | See also | 582625 | | Reporter | Adolfo Sanchez-Barbudo Herrera |

Description

MDT OCL has its Standard Library in an internal package. Since QVTo spec has its own implementation to extend the library with new types and operations, it would be useful for me having that OCLStandardLibraryImpl accessible in somehow:

  1. Moving the OCLStandardLibraryImpl to a non-internal package to be able to use that class.

  2. Creating a new extensible UML/EcoreOCLStandardLibraryImpl which would use the correspondent internal OCLStandardLibraryImpl.

Do you have any preference?

Cheers,\ Adolfo.

eclipse-ocl-bot commented 1 month ago

By Christian Damus on Dec 21, 2008 10:06

My preference is for the former. Why create more classes? Of course, we would probably like to clean up the existing class and expose only as much to subclasses as is really required. And probably there would be very little public API (mostly protected).

I think you already have a patch for this, is that right? When you have a patch ready, attach it and assign the bug to me.

eclipse-ocl-bot commented 1 month ago

By Adolfo Sanchez-Barbudo Herrera on Dec 22, 2008 06:20

(In reply to comment #1)

My preference is for the former. Why create more classes? Of course, we would probably like to clean up the existing class and expose only as much to subclasses as is really required. And probably there would be very little public API (mostly protected).

The main reason is that there are several public static methods which are used by internal classes (TypeResolver, UMLReflection, etc). Most of those methods aren't probably necessary to belong to new API.

One static class (the internal) can be considered as the stdlib builder, and the other (public one) can be considered as the OCLStandardLibrary implementation which only only provides implementation for that interface (and maybe few static methods).

I think you already have a patch for this, is that right? When you have a patch ready, attach it and assign the bug to me.

Yes I do. However I would like to clarify with Mariano some aspects related to the QVTo stdlib (some issues sent recently), before. There are some unclear points about the OCL stdlib must exactly be extended.

On the other hand, if option 1 is the preferred, I'll have to make an exhahustive of which of those methods are really needed. I want to be quite sure that we don't need any patch fater this one ;).

Can we wait until middle on january for this? API freeze is M6, isn't it?.

eclipse-ocl-bot commented 1 month ago

By Christian Damus on Dec 22, 2008 08:58

(In reply to comment #2)\

One static class (the internal) can be considered as the stdlib builder, and the other (public one) can be considered as the OCLStandardLibrary implementation which only only provides implementation for that interface (and maybe few static methods).

Yes, that makes sense. Not so much harm in a wrapper, I suppose.

Can we wait until middle on january for this? API freeze is M6, isn't it?.

That should be no problem. Although, if you post a patch sooner (with the understanding that it is not ready) then I will be more able to provide feed-back in my -- ahem -- copious spare time. :-)

eclipse-ocl-bot commented 1 month ago

By Adolfo Sanchez-Barbudo Herrera on Dec 22, 2008 10:57

Yes, that makes sense. Not so much harm in a wrapper, I suppose.

It can be as harmful as we want :D... I mean, we could have the current implementation as the OCLStandardLibraryBuilder(internal), which wouldn't extend OCLStandardLibrary anymore, and all their methods would be static.

The new EcoreOCLStandardLibrary would implement OCLStandardLibrary, and it would use that OCLStandardLibraryBuilder to obtain the types which were built by the latter.

The harm in this case, is that all the references to OCLStandardLibrary.INSTANCE would need to be replaced by the EcoreOCLStandardLibrary.INSTANCE, which I would be obviously very pleased to change ;P.

Anyway, just with the proposed wrapper would suffice to me.

Again, your decision .... The harmful or harmless ?

\

Can we wait until middle on january for this? API freeze is M6, isn't it?.

That should be no problem. Although, if you post a patch sooner (with the understanding that it is not ready) then I will be more able to provide feed-back in my -- ahem -- copious spare time. :-)

Ok, I'll provide the patch this week. If any further change would be needed due to specification reasons, I'll raise a new bugzilla one. Besides, I believe that we will have to do the hardest work when OCL 2.1 and QVT 1.1 come up.

eclipse-ocl-bot commented 1 month ago

By Adolfo Sanchez-Barbudo Herrera on Jan 08, 2009 06:44

Created attachment 121935 (attachment deleted)\ Proposed patch

Well, I have finally sent the harmless solution.

I have provided both, Ecore and UML based implementations.

Cheers,\ Adolfo.

eclipse-ocl-bot commented 1 month ago

By Christian Damus on Jan 10, 2009 17:22

Hi, Adolfo,

I have a few questions:

eclipse-ocl-bot commented 1 month ago

By Adolfo Sanchez-Barbudo Herrera on Jan 12, 2009 06:19

(In reply to comment #6)

Hi, Adolfo,

I have a few questions:

  • why define a static INSTANCE? That isn't extensible

That instance is the only way that an OCL client may use the standard library.

Anyway, for my needs, I don't need that instance becuase I have my own class. So feel free to remove it.

  • why is the getPackage() method static?

Again that's for allowing a client to get the unique package which the stdlib is built on without having to use the internal implementation. I have worked on the extension several times, and now I'm not requiring the use of the package itself, so that, you can remove it.

  • I noticed, when I hit Shift+Ctrl+F, that the text changed. Have you imported the formatter settings from the ocl.releng project? If you do, then the project-level preferences in the OCL projects will find it, so that code formatting in the OCL context is consistent. It won't affect any other projects in your workspace ... :-)

Mmm interesting point. Honestly, I'm not familiarized with this kind of preferences. Does this come from any Eclipse development best practices document? If so, could you give me its reference?. I have a little bit idea what the releng project is intended to, however I haven't worked with it and I'd be pleased to learn about it :)

BTW, I had a brief look at the impact of updating the EcoreEnvironment and UMLEnvironment implementations to add an instance variable for the stdlib, for which subclasses could provide their own implementation (based on your new public classes). There is still a considerable amount of code that depends on the static stdlib elements, which it would be very hard to change. So, I'm not sure what the practical benefit of this extensibility is?

Yep, this has been a big problem for me while working with the stdlib. I fortunately managed to solve all related problems working on the TypeResolver (for example adding new operations to the OCL predefined types) and the new TypeChecker, and this is the simplest patch for OCL Standard Library which I need to solve the QVT library one.

The real benefit is that I have a QVT StandardLibrary which replaces the OCL Standard Library and I don't have to manage the creation/load of all the OCL Standard Library types and operations, which are already managed by internal OCLStandardLibrary implementation.

Maybe for MDT-OCL 2.0 we could consider using the standard library in a non-statical way in all the code, so that an extending OCL standard library fits better in the implementation. I could help in this task ;)

In any case, this patch looks OK to me. Is it ready, or do you have more to add?

Yes, this patch would suffice. What about asking to Borland's team ?

Cheers,\ Adolfo.

eclipse-ocl-bot commented 1 month ago

By Christian Damus on Jan 12, 2009 08:44

(In reply to comment #7)

  • why define a static INSTANCE? That isn't extensible

That instance is the only way that an OCL client may use the standard library.

Ah, but what is wrong with, e.g., "new EcoreOCLStandardLibrary()" ? ;-)

  • The OCLStandardLibraryImpl still resides in an internal package.
  • The public Ecore/UMLStandardLibrary has a protected constructor, so that just an unique instance (of an extebsible class) exists.

Right, but the constructor could be public.

I'm just concerned about the usefulness of a singleton for a class that is meant to be extended. Also, the "official" way to access the stdlib is supposed to be through an Environment instance, so that clients needn't know whether it is a singleton or not.

Anyway, for my needs, I don't need that instance becuase I have my own class. So feel free to remove it.

  • why is the getPackage() method static?

Again that's for allowing a client to get the unique package which the stdlib is built on without having to use the internal implementation. I have worked on the extension several times, and now I'm not requiring the use of the package itself, so that, you can remove it.

Well, I won't remove it because the OCL parser, itself, sometimes looks at the stdlib package. I'll just change it to non-static to align with the accessors for this package's contained classifiers.

  • I noticed, when I hit Shift+Ctrl+F, that the text changed. Have you imported the formatter settings from the ocl.releng project? If you do, then the project-level preferences in the OCL projects will find it, so that code formatting in the OCL context is consistent. It won't affect any other projects in your workspace ... :-)

Mmm interesting point. Honestly, I'm not familiarized with this kind of preferences. Does this come from any Eclipse development best practices document? If so, could you give me its reference?. I have a little bit idea what the releng project is intended to, however I haven't worked with it and I'd be pleased to learn about it :)

Well, I don't know whether it is a "best practice" but Eclipse makes these settings shareable in source control specifically so that teams can standardize on them.

Because until recently I have been the OCL team, I standardized on the settings used by the UML2 component because they look good to me. :-)

Just use the Import button in the Java formatter preference page to import the OCLCodeFormatting.xml file in the org.eclipse.ocl.releng project. You don't need to set this profile as your (workspace-wide{ default. It just needs to be available for the project-level settings in the OCL projects to find it. It won't interfere with your other projects.

Maybe for MDT-OCL 2.0 we could consider using the standard library in a non-statical way in all the code, so that an extending OCL standard library fits better in the implementation. I could help in this task ;)

Oo! certainly, that would be welcome.

I had been hoping to implement the stdlib as a regular EMF-generated package, encapsulating the implementations of all the various operations within the generated code. The main problems that I see are:

I welcome ideas for solutions. :-)

In any case, this patch looks OK to me. Is it ready, or do you have more to add?

Yes, this patch would suffice. What about asking to Borland's team ?

I shall copy Radek for their input.

Thanks!

eclipse-ocl-bot commented 1 month ago

By Radomil Dvorak on Jan 12, 2009 09:49

(In reply to comment #8)

Hi, thanks for CC me!

We have not had a need for direct extensibility in terms of java interface of the OCL stdlib. I'm not aware it prevents an MDT OCL based QVT implementation to define the QVT Stdlib. So, I wonder if there is anything I have missed so far.

As Christian says, the access point via environment which we extend does not give any promise on the OCL stdlib being singleton or not.\ So I assumed, the QVT Stdlib can rather aggregate the OCL stdlib while we can choose whether to use the singleton way.

I just hope that Adolfo will use OMG mailing list to discuss the QVT Stdlib issues with Mariano ;)

eclipse-ocl-bot commented 1 month ago

By Adolfo Sanchez-Barbudo Herrera on Jan 13, 2009 10:29

(In reply to comment #8)

I'm just concerned about the usefulness of a singleton for a class that is meant to be extended. Also, the "official" way to access the stdlib is supposed to be through an Environment instance, so that clients needn't know whether it is a singleton or not.

Ok. I'm finally convinced :).

Oo! certainly, that would be welcome.

I had been hoping to implement the stdlib as a regular EMF-generated package, encapsulating the implementations of all the various operations within the generated code. The main problems that I see are:

  • Ecore EDataTypes do not support generalization
  • even for the UML stdlib, we would have to implement the companion classes for the data types because we convert to an Ecore representation in which data types can't own operations. UML2 maps DataType to EClass for this reason, so maybe we should just implement the entire stdlib as EClasses in both cases?

I welcome ideas for solutions. :-)

I suppose that you mean modeling the stdlib as an .ecore to take adavantage from the EMF generation, dont't you?. The problem is how to simulate the "metalevels" since the (unique) model instance of that possible metamodel must conform to the OCL metamodel. So I could imagine the following for the String M1 predefined primitive type.

From my point of view, it's a little bit odd doing these things. IMO, from the side of Ecore Metamodel binding, the oclstdlib must be a pure OCLEcore.ecore instance: an oclstdlib.oclecore file, so the corresponding classes in Java are created by the EcoreFactory (not by a new StdlibFactory which creates specialized classes).

Besides, considering EMOF as the metamode, in the OCL specification relates the need of creation of new Classes (at the same M1 level predefined type, with the SAME name of the related predefined DataType) in the OCL Standard Library, in order to specifically solve the fact the EMOF datatypes can't own operations.

It seems that it could be a good solution from the implementation side (having a specific java package for the library, generated classes, etc). However, I'm not sure it's the best solution from the specification side. Anyway, as we already know, the specification doesn't alwasy have the last word, since it may be incorrect.

I rememeber that Radek had its own idea about how to treat standard (and user-defined) libraries. Maybe he could share it's ideas about this :)

(In reply to comment #9)\

We have not had a need for direct extensibility in terms of java interface of the OCL stdlib. I'm not aware it prevents an MDT OCL based QVT implementation to define the QVT Stdlib. So, I wonder if there is anything I have missed so far.

So, I guess you still keep the OCLStandardLibraryImpl in your environments ?

I would like to have the OCL stdlib like an artifact which can be plugged-out from the MDT-OCL implementation (In this case, from the environment) and plug-in my own library implementation. Just using the same patterns applied to artifacts like environments, type resolvers, the type checker, etc would be interesting for language extenders.

The problem right now, is that MDT-OCL implementation was not initially thought to be an extensible language (at least not in terms of its Standard Library). So the MDT-OCL code is highly coupled with the standard library implementation (which could be good for OCL, but not for reusing it for extension), and that's the reason why using a new standardLibrary in the environment, is not completely necessary. Anyway, I hope that the implementation would be more flexible in that way.

Besides, Have you imagined that any predefined type from OCL (a collection or primitive type), would have to be replaced by new instance of its metatypes in the QVTo context. I hope that would never be required by QVTo spec. However, at least the implementation would be sufficient flexible to adopt this kind of changes.

I strongly believe using (extending, integrating, etc) the OCLStandardLibrary (or whatever extension) inside the implementation is not a bad decision. And the first step is having the possibility of extending the default OCL stdlib implementation :)

As Christian says, the access point via environment which we extend does not give any promise on the OCL stdlib being singleton or not. So I assumed, the QVT Stdlib can rather aggregate the OCL stdlib while we can choose whether to use the singleton way.

I just hope that Adolfo will use OMG mailing list to discuss the QVT Stdlib issues with Mariano ;)

Of course, I hope that the efforts we have been investing here, have an immediate result in the next realeses of both OCL/QVT spec :D. I have no doubts about we both have an acceptable implementations of the spec (mine for my phd, and your for the official Eclipse project ;) ).

BTW there are some issues related to QVTo Stdlib in the OMG mailing list, which haven't had too much (actually, any) feedback. Maybe you could send some comments there, so that the discussion were activated there.

Cheers,\ Adolfo.

eclipse-ocl-bot commented 1 month ago

By Christian Damus on Jan 14, 2009 22:59

(In reply to comment #10)

I suppose that you mean modeling the stdlib as an .ecore to take adavantage from the EMF generation, dont't you?. The problem is how to simulate the "metalevels" since the (unique) model instance of that possible metamodel must conform to the OCL metamodel. So I could imagine the following for the String

Yes, I suppose the question is what will the code generator think of instances of its metaclasses as extended by OCL? Good question. Certainly, it will not recognize the metaclasses that directly extend EClassifier ...

M1 predefined primitive type.

  • Creating an EClass (which could be called Integer) which extends PrimitiveType.
  • Include the required operations on it,

This is just like the "shadow classes" of the ungainly EMOF adaptation strategy defined in Chapter 13 of the spec.

Besides, considering EMOF as the metamode, in the OCL specification relates the need of creation of new Classes (at the same M1 level predefined type, with the SAME name of the related predefined DataType) in the OCL Standard Library, in order to specifically solve the fact the EMOF datatypes can't own operations.

Oo, I had the same thought. Don't you just hate these extra classes? Why did EMOF make the mistake of not allowing operations on data types? Of what use are data types without operations, anyway? Argh.

It seems that it could be a good solution from the implementation side (having a specific java package for the library, generated classes, etc). However, I'm not sure it's the best solution from the specification side. Anyway, as we already know, the specification doesn't alwasy have the last word, since it may be incorrect.

That's right. And, clearly, my ideas on the subject so far are quite early and not much developed. You raise some very good points. This is a good subject for discussion on the mailing list. Perhaps you would like to formalize some design proposal that we can use as a starting point for the discussion? Perhaps in collaboration with Radek, who you suggest has thoughts on the subject already?

On a more practical question, am I waiting for an updated patch on this bug? It seems that you and Radek have very different requirements in this area ...

eclipse-ocl-bot commented 1 month ago

By Adolfo Sanchez-Barbudo Herrera on Jan 15, 2009 11:35

That's right. And, clearly, my ideas on the subject so far are quite early and not much developed. You raise some very good points. This is a good subject for discussion on the mailing list. Perhaps you would like to formalize some design proposal that we can use as a starting point for the discussion? Perhaps in collaboration with Radek, who you suggest has thoughts on the subject already?

I have found the link about our discussion (nearly one year ago >.<)

http://dev.eclipse.org/newslists/news.eclipse.modeling.m2m/msg02380.html

I don't know if Radek has changed his mind about all those ideas ;P. I'll definitely open a discussion in the mdt.ocl-dev mail list.

\

On a more practical question, am I waiting for an updated patch on this bug?

For me, the proposed patch with the changes you mentioned is good for me.

It seems that you and Radek have very different requirements in this area ...

Definitely, since it seems we have different design approaches...

If it is not an inconvenience, I would like to have this enhancement, since along my code, I uniformily access the QVToStandardLibrary, which provides both QVT and OCL types, by Environment.getOclStandardLibrary contract.

Cheers,\ Adolfo.

eclipse-ocl-bot commented 1 month ago

By Adolfo Sanchez-Barbudo Herrera on Jan 15, 2009 13:38

Created attachment 122705 (Final?) Proposed patch

New patch which includes the changes commented above.

:notepad_spiral: ocl258973.txt

eclipse-ocl-bot commented 1 month ago

By Christian Damus on Jan 19, 2009 09:13

Committed the patch to HEAD (1.3 branch). Thanks, Adolfo!

eclipse-ocl-bot commented 1 month ago

By Ed Willink on May 27, 2011 02:48

Closing after over 18 months in resolved state.