Bugzilla – Full Text Bug Listing
| Summary: | Stereotype Profil | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Modeling] MDT.UML2 | Reporter: | TimeSquare Mising name <benoit.ferrero> | ||||
| Component: | Core | Assignee: | Christian Damus <give.a.damus> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | enhancement | ||||||
| Priority: | P2 | CC: | benoit.ferrero, Kenn.Hussey | ||||
| Version: | unspecified | Keywords: | plan | ||||
| Target Milestone: | M3 | Flags: | give.a.damus:
luna+
Kenn.Hussey: review+ |
||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | Community Support | ||||||
| Attachments: |
|
||||||
Created attachment 162422 [details]
propal to change
I have pushed changes to address this bug as a new branch bugs/300957, with one commit:
0d055d7c9ff6c2722f526117c72341d97fb9cbe8
This is not based on the attached patch.
More informative assertion-violation messages are now provided in the stereotype application and profile application API. JUnit regression tests for these messages are included.
Thanks, Christian! Here is my feedback: - the copyright date in org.eclipse.uml2.tests-feature/feature.xml needs to be updated - the messages in applyStereotype(...) and unapplyStereotype(...) begin with a capital letter whereas the others do not - the message for the case where there is no nearest package should probably be "element \"%s\" is not a package" (with the element passed as an argument to the formatter) - the delegation between the new and old getDefinition(...) methods is backwards IMHO; currently, the logic of the method needs to be maintained in two places whereas the if old method delegated to the new one (passing 'false' for the new argument), the logic would only need to be maintained in one place Also, FYI I have been removing obsolete CVS header comments like this one "on demand" as I make changes to source files: * $Id: ElementOperations.java,v 1.54 2010/09/28 21:02:15 khussey Exp $ (In reply to comment #3) > Thanks, Christian! Here is my feedback: Thanks for the detailed review, Kenn! > - the copyright date in org.eclipse.uml2.tests-feature/feature.xml needs to be > updated Oh, indeed. That's a different kind of copyright: it would actually show in the UI if somebody was brave enough to install the test feature into their workbench. > - the messages in applyStereotype(...) and unapplyStereotype(...) begin with a > capital letter whereas the others do not Odd inconsistency; I'll make them all lower-case. We don't need sentence case for programmer errors. > - the message for the case where there is no nearest package should probably be > "element \"%s\" is not a package" (with the element passed as an argument to the > formatter) I did it this way because the consequence is that the profile was not applied and elements in general don't have pretty qualified names, but I agree with your comment. An element not being in a package is actually an invalid model and we have UMLUtil::getQualifiedText(...) to provide nice names. > - the delegation between the new and old getDefinition(...) methods is backwards > IMHO; currently, the logic of the method needs to be maintained in two places > whereas the if old method delegated to the new one (passing 'false' for the new > argument), the logic would only need to be maintained in one place Yes, I did it this way because the code ends up being less messy with exit cases. Happily, these methods are static, so we don't have to worry about delegating to the old method because it may have been overridden in client code. I'll try it the other way around and see how it looks. > Also, FYI I have been removing obsolete CVS header comments like this one "on > demand" as I make changes to source files: Yes, I have been, too. I just missed this case. :-/ I have pushed a new commit fd32073 to address the code review comment #3. Also, because now the pre-existing UMLUtil::getDefinition(Element, Stereotype) method is updated to delegate to the new API, I added some assertions in the test cases to check that it still works as before. Ordinarily I am very particular about using braces in if/else conditionals and loops, but I forego that in the case of the new UMLUtil::getDefinition(...) method because it really does make it less readable to have essentially superfluous braces on every occurrence of the same form of throw/return alternation. I hope that's OK. (In reply to comment #5) > Ordinarily I am very particular about using braces in if/else conditionals and > loops, but I forego that in the case of the new UMLUtil::getDefinition(...) > method because it really does make it less readable to have essentially > superfluous braces on every occurrence of the same form of throw/return > alternation. I hope that's OK. I added a "return null;" to the end of the method and removed it from all of the various places where exceptions were being thrown (adding braces in the process). The changes have been merged and pushed to the 'master' branch in git. The changes are available in an integration build for 4.2.0. |
Build Identifier: When apply a stereotype to uml element , org.eclipse.uml2.uml.Element element=.... org.eclipse.uml2.uml.Stereotype stereotype= ... element.applyStereotype(stereotype); if isn't possible to apply a stereotype methode launch a IllegalArgumentException(String.valueOf(stereotype)). is it possible to have a more explicit message ? ex: The profil is not apply to the neaster package .... Reproducible: Sometimes