groupBy(shadow entity) should throw error + support groupBy(identity(shadow entity))

Proposal B1) just add the warnings for CS
+
Proposal C2)
   groupBy(integer) // equality
    groupBy(identity(shadow variable)) // identity
    groupBy(identity(shadow variable), integer) // identity, equality
    groupBy(integer, identity(shadow variable)) // equality, identity
+
Proposal X) Add a TestData objects that mimics this behavior
class TestDataHashCodeChangingEntity
  shadow var that in is hashcode
tests:
    groupBy(shadow entity) // Identity would fix it, equality breaks it
    groupBy(integer)       // Equality would fix it, identity breaks it
    groupBy(identity(shadow variable), integer

Make @PlanningId mandetory

Proposal D) Detect usage of Lombok
-1 it's not just lombok

Proposal A) Do nothing.
There are no breadcrumbs.

Why isn't this a problem with Drools?
It probably is.

Proposal X) Add a TestData objects that mimics this behavior
class TestDataHashCodeChangingEntity
  shadow var that in is hashcode
AGREED
    groupBy(shadow entity) // Identity would fix it, equality breaks it
    groupBy(integer)       // Equality would fix it, identity breaks it


Proposal B) Try to find all exceptions for which this would bubble up as a score corruption or a move corruption or a "impossible state" and add a line "Maybe your hashcode implementation is (...messed up)"
"Maybe the hashCode is ..." (a few false positives are fine)
B1) only in that CS code. Better than A). EAsy (in Uni, Bi, Tri, Quad)
B2) also figure out for non-CS code. Avoid too many false positives. => killer, maybe just go for B1)


Proposal C) Experiment with IdentityHashMap or such semantics
  groupBy(shadow entity) VS groupBy(integer)
  =>
  C1) groupByIdentity(shadow entity)
      groupBy(integer)

      multi parameters?
      groupByIdentityForFirstParam(shadow entity, integer)

Theory: only have this problem for JDK class:
groupBy(integer) = groupBy(String)
Equality for integer, string, localtime, MyLocation(lat, long), ...
Identity for @PlanningEntity (including shadows),

ann = new Person("ann", LocalTime.of(9:00))
beth = new Person("beth", LocalTime.of(9:00))
groupBy(Person::getTime, count())=> requirement: 1 group: {9:00, 2}

=> Conclusation: equality semantics must be the default (80% rule)

Should it be the only path? principle of least surprise?


Proposal E) Make @PlanningId mandatory

Proposal F) Detect if a PlanningEntity is implement equals and throw an exception if that happens
Stronger breadcrumb than B1
=> if they don't want to change their domain objects => no optaplanner for you



Proposal D) If class has @PlanningId, use that as groupBy, otherwise use equality semantics



Proposal B1) just add the warnings for CS
=> if they don't want to change their domain objects => no optaplanner for you

Proposal B1) + C2)
C2) groupBy(integer) // equality
    groupBy(identity(shadow variable)) // identity
    groupBy(identity(shadow variable), integer) // identity, equality
    groupBy(integer, identity(shadow variable)) // equality, identity

=> error message: "maybe fix your hashcode. If you really don't want to, wrap it in Collectors.identity()"

Con: suffer in scoreDRL
     I don't want to know as a user.


Proposal E + F: force @PlanningId
if (@PlanningEntity) {
    if (no @PlanningId) throw exception
    equality on planning Id
} else {
    equality
}
Violates principle of least surprise.

Version 1 of MyProject: Computer is not an PE, in version 2 Computer becomes a shadow entity
because of some new constraint
groupBy(Process:getComputer) from version 1.
Violates principle of least surprise.


Optional PlanningId + change groupBy behavior on @PlanningId detection.
-> because behavior existing constraints changes when adding PlanningId for multithreaded solving

Mandetory PlanningId => getting started is more difficult