Fix equals and hash code variable

[Johansogaard]

I encountered an issue where VariableId was used as a key in the HashMap inside VariableSnapshotTotal, but the key couldn't be found — even though, in the debugger, the objects inside the VariableId record looked exactly the same (also memory location). The hash also appeared correct in the debugger.

My solution was to change the equals() method in VariableId to use .equals() on both fields instead of ==. If the user doesn't override equals() for their entity, it still behaves the same as before. However, this change enables users to define logical equality themselves.

I believe part of the issue came from the default hashCode() behavior. Since Java's default hash is based on memory addresses, it can lead to bad hash distributions — especially when two logically identical objects (like two copies of an entity with the same ID) are placed into different buckets. I also had to implement proper equals() and hashCode() in my planning entity, which I noted in a comment in the code. After doing that, Timefold started working correctly again.

My guess is that the entity is cloned or copied at some point and for some reason my debugger wasn't showing me the correct memorylocation, and that's why some VariableId instances couldn't be found

I was implementing moveSelectors when this became an issue.

If the original design was intended to enforce identity equality for specific reasons, or if you have an idea what happened for me to have this issue let me know it works now following my approach overwriting hash and equals in entity using a better value for hashing (like id) :)

[Copilot]

Pull Request Overview

This PR fixes the logical equality issue in the VariableId record by modifying the equals() method to use .equals() on both the variableDescriptor and entity fields. It also adds documentation to emphasize the need for planning entity classes to implement proper equals() and hashCode() methods based on a unique identifier.

• Updated equals() method to use .equals() instead of ==.
• Updated documentation to clarify the requirement for overriding equals() and hashCode() in planning entities.

Comments suppressed due to low confidence (1)

core/src/main/java/ai/timefold/solver/core/impl/domain/variable/listener/support/violation/VariableId.java:18

• Since equals() has been overridden to use .equals() on both fields, please ensure that hashCode() is also overridden to maintain the equals-hashCode contract.

public record VariableId<Solution_>(VariableDescriptor<Solution_> variableDescriptor, Object entity) {

[triceo]

Hello @Johansogaard, and thank you for your contribution!

It is generally frowned upon to compare entities by their identity - precisely because of cloning. No two entities within the same solution are allowed to equal, and no two entities from different solutions should ever be compared to each other. In fact, ScoreDirector has methods for looking up entities from different score directors, lookup(...).

So, the correct way for VariableId to handle this issue would have been to first lookup the correct entity, and only then actually create the instance of the Id. I am not currently sure why that's not the case, and I won't be able to look into it until I come back from my PTO. But I will find out what's going on here when I'm back.

In the meantime - would you mind explaining how you got here? What were you doing that this code failed for you?

[Johansogaard]

Sounds good. This was the error I was getting:
Cannot invoke "ai.timefold.solver.core.impl.domain.variable.listener.support.violation.VariableSnapshot.isDifferentFrom(ai.timefold.solver.core.impl.domain.variable.listener.support.violation.VariableSnapshot)" because "variableBefore" is null.

I found the issue to be that the variableId could not be found in the HashMap in VariableSnapshotTotal for two variableIds — it could find it for the rest.

I can reproduce the error by removing the overridden hashCode and equals methods in the entity class, so the equals I made in this pull request might not have anything to do with it.

In the debugger, the variableId in this.variableIdToSnapshot contains exactly the same objects as in before.variableIdToSnapshot — both with the same variableDescriptor and entity — but still, the get() call returns null from the before HashMap.

This is all fixed when i overwrite the hashcode and quals in entity class

[Christopher-Chianelli]

Since it appears you are implementing custom moves; did your custom moves add entities? For instance, did you do something like this?

var newEntity = new Entity();
scoreDirector.beforeEntityAdded(newEntity);
solution.getEntities().add(newEntity);
scoreDirector.afterEntityAdded(newEntity);

or alternatively, something like this:

var oldEntity = solution.getEntities().get(0);
var newEntity = new Entity(oldEntity.id());

scoreDirector.beforeVariableChanged(newEntity, "variable");
newEntity.setVariable(1);
solution.getEntities().set(0, newEntity);
scoreDirector.afterVariableChanged(newEntity);

Currently adding or replacing entities is unsupported in a Move is unsupported.