Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

method .is("*") returns different results for visual objects an concept #122

Open
markbacker opened this issue Jul 18, 2023 · 7 comments
Open

Comments

@markbacker
Copy link

markbacker commented Jul 18, 2023

Version of jArchi, Operating System

  • Archi: 5.0.2.202304051112
  • Platform: windows 10
  • Javascript engine: GraalVM
  • jArchi plugin: 1.4.0.202303211027

Expected Behaviour

A test with .is("*") should return the same result for both visual objects as for concepts.

So far, I've seen this behavior only with the selector "*".
This didn't happen in a previous release. Don't know the jArchi version that introduced this error

Actual Behaviour

The test with a visual object returns false.
The test with the same object selected in the model tree returns true

Steps to Reproduce the Behaviour (with attached test files/script)

/**
 * test .is()
 * 
 * select an application-component on a view and run the script
 * next select the application-component in the model tree and run the script
 * 
 */
console.log(`Result from .is("*") = ${$(selection).is("*")}`)
console.log(`Result from .is("application-component") = ${$(selection).is("application-component")}`)
@Phillipus
Copy link
Member

Not sure about this.

The relevant part of the code is this:

// All model concepts, diagram models, and folders
if(selector.equals("*")) { //$NON-NLS-1$
return new ISelectorFilter() {
@Override
public boolean accept(EObject object) {
return (object instanceof IArchimateConcept || object instanceof IDiagramModel
|| object instanceof IFolder);
}
};
}

This means that If the object is a diagram object (visual object) then it returns false, and true if a concept (in the model tree)

@jbsarrodie WDYT?

@markbacker
Copy link
Author

But why different behavior for the "*" selector?

The test with an application-component selected on a view gives:
Result from .is("*") = false
Result from .is("application-component") = true
Result from .is("element") = true

@Phillipus
Copy link
Member

Phillipus commented Jul 18, 2023

Because the is function ultimately calls the code I quoted above. The "*" selector only returns true for concepts, views and folders.

I'm not sure whether this should be changed or not.

@jbsarrodie
Copy link
Member

@jbsarrodie WDYT?

This is the intended behavior: is() check a collection against a selector. Thus collection.is(selector) returning true implies that collection.filter(same_selector) is not empty.

The intended behavior for "" selector is to return only ArchiMate concepts and "primary" containers (folders and view). The reason for that is that we want some selector to return actionable content and not thousands of visual objects (they can be accessed by other means). That's what "" is for. So a collection containing only visual objects won't match the "*" selector, and in this case is("*") returns false.

In fact, the real question should be why "application-component" (or "element", or any specific selector) works on visual objects. The reason for this is that as soon as you've selected some visual objects (related to ArchiMate concepts), in almost all cases, you'll want to check the type of the underlying concept and not the visual object itself (and same is true for properties), so we've made it easier for people, and in this case jArchi checks against .concept and not the visual object. But the drawback is that we had to make sure that $(selector) (which basically is model.find(selector)) doesn't return also visual objects, so we've implemented it as model.find(selector).filter("*")

@markbacker
Copy link
Author

Thanks for the clear explanation. This also helps in understanding the reasoning behind why visual objects do have properties. And true, it does give shorter code.

Having said that, I don't think I agree with this choice, and I don't think it makes it easier for people. For me, this is confusing. I have made many mistakes in using object and object.concept ...

Now I know how to solve the error in my script. I will use the condition if ($(obj).is(selector) || selector == "*")

@Phillipus
Copy link
Member

Having said that, I don't think I agree with this choice, and I don't think it makes it easier for people. For me, this is confusing. I have made many mistakes in using object and object.concept

How do you think it could be implemented differently?

@markbacker
Copy link
Author

I would prefer that the API is explicit about whether you are working with a visual object or an object.

  • obj.prop() is only applicable for objects; visual objects don't have properties. To access the object, you have to use the .concept method
  • Just like only visual objects have the method .fontname().
  • If you use obj.id and obj.prop(), you can be sure you are getting the values of an object (and if you mistakenly pass a visual object, you get an error on the .prop() method).

And for the .is() method.

  • .is("*") returns everything
  • .is("element") return all elements (objects and visual objects)
  • .is("concept") returns all elements and relations (a bit confusing with the .concept method. A more explicit name would be simply "element and relation")
  • to explicitly choose for object or visual objects there are also selectors for .is("object") (all objects in the model tree, equal to the "*" selector now) and .is("visual-object"). Combining these with the previous selectors makes all/most selections possible
  • and a selector .is("diagram-object") to select diagram-model-note, group, etc.

For me this is more consistent and less prone to errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants