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

Allow mixed-type equals #249

Open
mhrimaz opened this issue Feb 15, 2024 · 3 comments
Open

Allow mixed-type equals #249

mhrimaz opened this issue Feb 15, 2024 · 3 comments

Comments

@mhrimaz
Copy link

mhrimaz commented Feb 15, 2024

I created a separate issue for the PR #248 and allowing mixed-type equals which also mentioned in the community meeting:
https://www.eclipse.org/lists/aas4j-dev/msg00041.html

Metamodel elements defined as interfaces (like public interface Property extends DataElement) and aas4j allows to have various concrete implementation. The most important advantage is to allow someone to have a more efficient implementation (use linked list instead of array list, or keep some internal states for more efficient navigations,...). However, if we have different implementation of an entity (e.g. Property), should we allowed to compare them?

The mentioned article shed lights on many logical challenges:
http://www.angelikalanger.com/Articles/JavaSolutions/SecretsOfEquals/Equals-2.html

As mentioned in the article not allowing mixed-type makes life easier. But we can also see other examples that allow it.
As an example the equals for java ArrayList allows any subtype of List for comparison. So you can compare LinkedList with ArrayList.

List<String> one = new ArrayList<>();
one.add("test");
List<String> two = new LinkedList<>();
two.add("test");
assert one.equals(two);

So the most important thing is to define a clear logic and semantic as it might get very tricky.

For example we have AnnotatedRelationshipElement and RelationshipElement. Should equals be always false? or if AnnotatedRelationshipElement has no annotation, then it can be comparable to RelationshipElement.

@mjacoby
Copy link
Contributor

mjacoby commented Feb 15, 2024

If we go for mixed-type equality, can we assure that all (future) implementation of an interface like Property will behave in the same expected way or is this up to the developer of each of those classes? If we can't ensure this, might this be a problem?
Does this even require major effort on our site to implement/support this as we only provide a single implementation of each interface?
In general I like the idea of allowing mixed-types equality but only if it makes sense and we don't find major downsides (besides complexity).
One potential downside I can think of is that currently the model code, i.e. also the equals methods, are auto-generated by our generator tool and I am not sure if such a complex pattern could also be auto-generated.

RelationshipElement and AnnotatedRelationshipElement are a special case and I guess an instance of AnnotatedRelationshipElement should never be considered equal to an instance of RelationshipElement. Reasoning is, that if you follow through with this logic you should also consider an empty Property equal to an empty Range element because both are instances of SubmodelElement and in this case they would ne differ in attributes.

@mhrimaz
Copy link
Author

mhrimaz commented Feb 16, 2024

If we go for mixed-type equality, can we assure that all (future) implementation of an interface like Property will behave in the same expected way or is this up to the developer of each of those classes? If we can't ensure this, might this be a problem? Does this even require major effort on our site to implement/support this as we only provide a single implementation of each interface? In general I like the idea of allowing mixed-types equality but only if it makes sense and we don't find major downsides (besides complexity). One potential downside I can think of is that currently the model code, i.e. also the equals methods, are auto-generated by our generator tool and I am not sure if such a complex pattern could also be auto-generated.

We can't control how people implement their equals, but all we can is to provide examples and sample test cases to guide them through a correct implementation.
I also don't say definitely that we should have such attribute for equals. The motivation was to test if a CustomProperty can be serialized with RDF serializer, but the reconstructed object for now is always DefaultProperty. So I can't compare them using equals method right now.

RelationshipElement and AnnotatedRelationshipElement are a special case and I guess an instance of AnnotatedRelationshipElement should never be considered equal to an instance of RelationshipElement. Reasoning is, that if you follow through with this logic you should also consider an empty Property equal to an empty Range element because both are instances of SubmodelElement and in this case they would ne differ in attributes.

I also think so. In this case, I need to modify also the current PR if we want to have such behaviour.

@mjacoby
Copy link
Contributor

mjacoby commented Feb 16, 2024

The motivation was to test if a CustomProperty can be serialized with RDF serializer, but the reconstructed object for now is always DefaultProperty. So I can't compare them using equals method right now.

Another possible solution for this problem would be to add a method similar to useImplementation(Class<T> aasInterface, Class>? extends T> implementation) in JsonDeserializer.java#L59 also for RDF.
I think this would be very beneficial. And if you have something like this it would solve your problem related to testing.

However, this would not answer the question if we should change how equals work in AAS4J (or in certain classes at least).

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

2 participants