Details
-
Task
-
Resolution: Obsolete
-
Major
-
None
-
None
-
None
-
NEW
-
NEW
Description
Highlights of decisions: we're implementing proposal B1 + C2 + X:
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
Long discussion:
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