#629 Vertical scaling from finalized UML model to guard computation#686
#629 Vertical scaling from finalized UML model to guard computation#686AndreaPuffo wants to merge 6 commits into
Conversation
- Do not simplify control flows if they have guards - Do not simplify decision/merge nodes if they represent initial/final nodes
with double underscores later in the synthesis chain.
| * @return The new element's name. | ||
| */ | ||
| private static String getNewElementName(RedefinableElement umlElement, Activity activity) { | ||
| if (umlElement instanceof OpaqueBehavior || umlElement.eContainer().equals(activity)) { |
There was a problem hiding this comment.
In what situations will umlElement be an OpaqueBehavior? (I don't immediately see a call of getNewElementName in this PR with an opaque behavior argument.)
There was a problem hiding this comment.
It's just for completeness, in case we wanted to use this method also in other places/situations. Alternatively, I can throw an exception and deal with this case if it is ever the case.
There was a problem hiding this comment.
Ok, I see. The JavaDoc of getNewElementName states "Creates a new name for the given UML element and its container activity". Do we support opaque behaviors that are defined inside activities at the moment? If not, then I'd either change the JavaDoc of this method to document that getNewElementName can also be used for other elements, or I'd remove the OpaqueBehavior case.
There was a problem hiding this comment.
No, we don't support opaque behaviors inside activities, just within the model. I'll change the JavaDoc and also the method itself.
There was a problem hiding this comment.
I realized I made a mistake and my explanation was lacking some details. This is what's happening: we have the synthesized activity, which can contain newly generated elements (think of CallBehaviorAction that calls an OpaqueBehavior) and elements that are duplicated from a called concrete activity. The latter elements have names with double underscores, which we don't allow.
Then we do this:
- for each node in the synthesized activity, we use the tracker to get the original element which the current node refers to (e.g., for a
CallBehaviorActionwe get anOpaqueBehavior, for a duplicate node we get the node contained in the concrete activity) - if the original node belongs to a called concrete activity, we should change its name; if not, keep its name as is. For example, in this latter case the original element would be an opaque behavior
I will change the method to better reflect this explanation.
There was a problem hiding this comment.
Thank you for the explanation, I now better understand the use of getNewElementName. I wrote a few separate follow-up comments.
Co-authored-by: Wytse Oortwijn <wytseoortwijn@gmail.com>
|
|
||
| /** | ||
| * Creates a new name for the given UML element and its container activity, without the use of double underscores. | ||
| * Creates a new name for the given UML element if it does not belong to the synthesized activity. The new name is |
There was a problem hiding this comment.
Could we make the JavaDoc slightly more accurate, by changing it to something like: 'Creates a new name for the given UML element in case that element belongs to an activity but not the given synthesized activity.'? Then it's clearer from the documentation in what situations a new name is being returned.
| */ | ||
| private static String getNewElementName(RedefinableElement umlElement, Activity synthesizedActivity) { | ||
| if (umlElement.eContainer() instanceof Activity && !umlElement.eContainer().equals(synthesizedActivity)) { | ||
| return ((Activity)umlElement.eContainer()).getName() + "_" + umlElement.getName(); |
There was a problem hiding this comment.
I now get that this line returns a new name for element that doesn't contain a double underscore. But doesn't umlElement.getName() still have the double underscore then? (I'm probably missing something obvious here.)
| * @return The new element's name. | ||
| */ | ||
| private static String getNewElementName(RedefinableElement umlElement, Activity synthesizedActivity) { | ||
| if (umlElement.eContainer() instanceof Activity && !umlElement.eContainer().equals(synthesizedActivity)) { |
There was a problem hiding this comment.
I propose to change this code to:
if (umlElement.eContainer() instanceof Activity activity && !activity.equals(synthesizedActivity)) {
return activity.getName() + "_" + umlElement.getName();
}| * @return The new element's name. | ||
| */ | ||
| private static String getNewElementName(RedefinableElement umlElement, Activity activity) { | ||
| if (umlElement instanceof OpaqueBehavior || umlElement.eContainer().equals(activity)) { |
There was a problem hiding this comment.
Thank you for the explanation, I now better understand the use of getNewElementName. I wrote a few separate follow-up comments.
Update the finalization of opaque actions (step 15 of the synthesis chain). Renames UML nodes to remove double underscores (which will cause issues in later steps of the synthesis chain).
Updates the simplifications of the synthesis chain (step 17) to deal with concrete elements, namely control flows with non-trivial guards and decision/merge nodes which represent initial/final nodes of concrete activities.
Closes #629. Addresses #410.