Community
Participate
Working Groups
Build ID: M20071023-1652 Steps To Reproduce: Hello, If I annotate my setter in my interfaces with @model the generated model code contains duplicate methods. Best regards, Lars ------------- Example: ----------------------------- package test; /** * * @model */ public interface Testing { /** * * @model */ public String getName(); /** * * @model */ public void setName(String value); } ---------------------------- Create a new model based on this interface and create the model code from it. More information:
Created attachment 86094 [details] Patch to addess the problem. We can't simply assume that all operations defined by a class should be generated into the interface since they might collide with signatures of accessors for features of the class itself. The patch does that extra filtering...
Dave, The patch contains other changes pending for this class, but the basic change getDeclaredGenOperations are quite simple. Does this look okay to you? (I need to attach a new patch since I did just notice a problem...)
Created attachment 86095 [details] Slight fixup
First off, does Lars understand that what he's doing here is explicitly declaring an operation named setName in his class, when a method with that name will already be generated for the name feature that he has declared with the getter? Second, is there any reason why this should be valid? If not, my preference would be to add validation to catch this. I'd imagine that doing this kind of thing would be a mistake 99% of the time, so if there's no compelling reason to allow it, I'd rather flag it so that people fix their models.
Hi, validation would also be ok. The way I run into this, was where I had a large interface and started pasting @model to each method including the setter. Best regards, Lars
Dave, it seems a bit odd to me to consider it invalid. It's redundant for sure, but invalid seems overly strong. To me it seems best to be tolerant of it in the generator...
I know we're tolerant of operations that are redefined in subclasses by features, but I'm having trouble coming up with any justification to allow a single class to declare two different model elements that lead to generating potentially conflicting pieces of code (don't forget that the operation could be annotated with a body). If there's no legitimate use for this (i.e. if it doesn't buy you anything), then I just can't see why we should consider it valid. It seems so much better to tell people they're making a mistake then to ignore it. Just thinking of a large model with each feature duplicated by a pair of operations makes me feel ill. Why tolerate that?
All that being said, can you really argue that it's literally wrong as opposed to just being redundant or a form of obscenity, i.e., something for which you'd produce a warning? :-P I'm curious if Christian has an opinion on this from an OCL perspective...
I can certainly argue that it's literally as wrong and/or redundant as the resulting Java code. Which, of course, produces an error and refuses to compile. ;) Adding more code to silently remove the wrongness makes about as much sense to me as "enhancing" the Java compiler to ignore the second method declaration. It seems a much better use of effort to add an error that explains the wrongness to the user in terms of the model. Again, I take this hard-line stance only if there is no legitimate use for such a construct.
Since I've been cc'ed into the fray ... I would expect that @model on a setter method would result in an EOperation being defined in the Ecore model because: - the convention is that @model on a bean-style getter indicates an EStructuralFeature - Ecore has read-only properties (in which case there is only a getter) but not write-only properties (only a setter) - a user typing @model in the Javadoc comment can only reasonably be interpreted by the EMF tooling as a deliberate attempt to tell it something important, not a mistake (which the original scenario appears to have been) Of course, this then begs the question of how to use @model to indicate that the getter should also produce an EOperation in the Ecore model, in addition to the EStructuralFeature. Because, of course, in Java we can't have two declarations of the method and we also don't have fields in interfaces that we could tag with @model. So, I tend to agree with Dave that @model on the setter should produce a warning and be ignored. Otherwise, I would expect it to generate an EOperation for *both* the setter and the getter (what use is the former without the latter?), but this isn't really feasible because I would have no way to get the EOperation for a read-only property. There's no other clean way out of this can of worms until EMF implements Java 5.0 annotations, which could then express all of these cases. OCL doesn't enter into the question. OCL only cares about the model, not the Java implementation.
I agree with Christian, that the @model tag on the setter tells EMF that there is an operation. There is as well a way to tell EMF that the getter is an operation. I can look it up, its something like operation=true If the getter has an @model tag, and not the tag telling that this is an operation, the getter tells EMF that there is a feature, and if the feature is changeable=true, then there is duplicate code. Thus what is needed would be, that - the "Y getX()" operation is identified as conflicting with the X feature of type Y - the "setX(Y v)" operations is identified as conflicting with the X feature of type Y, BUT only if the feature has changeable=true Only in these two cases, the code should be marked as illegal. As well, it should only be marked as illegal, if this happens in the same class. There are numerous examples on the newsgroup, showing that declaring getter and setter as operations in a super class, and then implementing it in a subclass as feature makes a lot of sense. In a recent post, Ed Merks explicitly recomends, to define getter and setter in ECLasses with interface=true as operations, in order to decide freely in the implemeting EClasses, whether to implement them as operations or features.
Since everyone agrees, I'll work on constraints instead. Things like unsetX and isSetX will need attention as well. Thanks for all the input.
Created attachment 86301 [details] Implement constraints instead I've implemented the constraint by creating a set of operations that correspond to the accessor methods that would actually be generated in the interface and then reusing the existing constraint code that operation signatures within an EClass ought not to collide. I've modified Bad.ecore to contain both an example of all the violations as well as one that might look incorrect but isn't because the accessors are suppressed. Does this look correct?
Looks good, Ed. The one thing that looks a little bit fishy is that you're setting the eType of the EOperation for the getter, but that's not actually checked in the comparison (nor is it set for the isSet operation).
Dave, That's true, but in the future we might add more constraint checking on covariant return types and such so it seemed best to generate the exact signature. Mind you, I can think of many problems such as will it be EList verses List... Do you think it's best to remove/comment-out it for now?
What made me notice was that you're not setting the type of the isSet operation to EBoolean. So, if the intent was to generate the exact signature for possible future use, I'd do that one too.
The changes are committed to CVS for 2.4.
Fixed in I200801090807.
(In reply to comment #8) > All that being said, can you really argue that it's literally wrong as opposed > to just being redundant or a form of obscenity, i.e., something for which you'd > produce a warning? :-P > > I'm curious if Christian has an opinion on this from an OCL perspective... > I agree with Ed that operations whose signatures conflict with feature accessors should not be considered invalid... Ecore is not Java... if I were looking to create an Ecore/EMOF model and never generate code from it, why should I have to appease constraints that are clearly biased toward generation of code in a specific language (Java) using a specific code generator (EMF's)?
(In reply to comment #7) > I know we're tolerant of operations that are redefined in subclasses by > features, but I'm having trouble coming up with any justification to allow a > single class to declare two different model elements that lead to generating > potentially conflicting pieces of code (don't forget that the operation could > be annotated with a body). > > If there's no legitimate use for this (i.e. if it doesn't buy you anything), > then I just can't see why we should consider it valid. It seems so much better > to tell people they're making a mistake then to ignore it. Just thinking of a > large model with each feature duplicated by a pair of operations makes me feel > ill. > > Why tolerate that? > It's actually a common pattern in UML models to declare an operation with a signature that conflicts with the accessor for a derived feature so as to declare how the feature's value is derived... Also, in UML2 we use such operations to override the default getter/setter behavior for certain features...
(In reply to comment #17) > The changes are committed to CVS for 2.4. > These changes have caused the Ecore model for UML2 to become "invalid". :( After discussing this with Ed, he agreed to relax the constraint so that it is ignored in cases where the offending operation has suppressed visibility... which means that support for supression of operation visibility needs to be added to EMF - see bug 215407. Once this support has been added, the Ecore profile in UML2 will need to be extended so that operations can have a visibility tag, the Ecore converter will need to be enhanced to process these new tags, and the UML2 code generator will need to be updated accordingly. Talk about a ripple effect...
Move to verified as per bug 206558.
