maestro
maestro copied to clipboard
Cannot compare on PType
PType is used as key in HashMap in several places in the fmi2api
project, i.e. ioBuffer
, sharedBuffer
.
However, PType
implementers only overrides equals
and not hashCode
. Therefore, it does not work as regular keys in HashMap
and requires additional functions such as https://github.com/INTO-CPS-Association/maestro/blob/development/frameworks/fmi2api/src/main/java/org/intocps/maestro/framework/fmi2/api/mabl/variables/ComponentVariableFmi2Api.java#L127-L137
For example, the groupingBy collector does not allow for comparison on PType.
PType a = new ARealNumericPrimitiveType();
PType b = new ARealNumericPrimitiveType();
PType[] array = {a, b};
Map<PType, Long> collect = Arrays.stream(array).collect(Collectors.groupingBy(Function.identity(), Collectors.counting()));
System.out.println(collect);
prints {real=1, real=1}
but the ideal would be {real=2}
such that they are grouped together.
- Shall we provide
hashcode
for thePType
implementors? - Shall we use
TreeMap
instead ofHashMap
? (worse in terms of performance...) - Kenneth, you come up with ideas! :)
Can we safely override hashcode?
@CThuleHansen I don't know the background to this one, but if you change the hashCode for a type, you have to be sure that the various Java contracts are honoured, otherwise you get some very peculiar errors. For example, hashCode and equals must be consistent; you also have to be careful about compareTo (especially if that uses equals). If you get this wrong, then complex aggregates of these types (typically maps) start to produce unexpected results.
@nickbattle thank you very much for giving advice! I think this is the main cause of our issue - equals is overridden but hashcode is not! So we are using PType as key in some maps. Here is an example with ARealNumericPrimitiveType which extends PType (somehwere up the tree) Example:
public class ARealNumericPrimitiveType extends SNumericPrimitiveBase
{
/**
* Essentially this.toString().equals(o.toString()).
**/
@Override
public boolean equals(Object o)
{
if (o != null && o instanceof ARealNumericPrimitiveType) {
return toString().equals(o.toString());
}
return false;
}
/**
* Forwarding hashCode call to {@link Object#hashCode()}.
**/
@Override
public int hashCode()
{
return super.hashCode();
}
This is ASTCreator stuff, anything you are familiar with? Kenneth is on vacation :)
What sort of Map are you using? It makes a difference: if you're using TreeMaps, then the compareTo method contract must be respected; if you're using HashMap, then the equals/hashCode contract must be respected (though it's generally a good idea to respect EVERY contract!).
(The problem in VDMJ was that the compareTo method of Values is effectively user-defined, if you include an "ord" clause on a type. But that meant that you could write VDM that causes Java's map library to fail to find entries in the map!)
@nickbattle hashmap
. But the issue is also that the stream groupingby collector
also uses the hashcode
. So there is a lot lost because of this
The HashMap uses the hash of the key to locate a bucket, and then uses equals to find the item in that bucket. But, depending on the hash, you might end up putting an item in the wrong bucket and then it cannot locate it again when "the same" key is used later (ie. similar, but not "the same", like a Long or an Integer of the same value might have different hashes?).
If the equals is using toString, the hashCode probably ought to use that too (ie. return toString().hashCode())
I agree with @nickbattle not sure why this is not the case. Like the mismatch here can lead to pretty strange behaviour so will a fix. It looks like the ASTCreator it self relies on the hashcode being unique per object (not the equal based on string) for making sure it doesn't visit the same node twice during a search.
So it seems like we should make a utility method for this...