[ https://issues.apache.org/jira/browse/OFBIZ-10593?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16638374#comment-16638374 ]
Mathieu Lirzin edited comment on OFBIZ-10593 at 10/4/18 3:20 PM: ----------------------------------------------------------------- I have uploaded [^OFBIZ-10593_Implement-EntityConditionVisitor-properly.patch] which is a WIP patch trying to improve the situation by using the following {{EntityConditionVisitor}} interface: {code:java} public interface EntityConditionVisitor { <T extends EntityCondition> void visit(EntityJoinOperator op, List<T> conditions); <L extends EntityCondition, R extends EntityCondition> void visit(EntityJoinOperator op, L lhs, R rhs); <L,R,T> void visit(EntityOperator<L, R, T> op, L lhs, R rhs); <L,R> void visit(EntityComparisonOperator<L, R> op, L lhs, R rhs); void visit(EntityConditionValue.ConstantNumberValue value); void visit(EntityFieldValue value); void visit(EntityExpr expr); <T extends EntityCondition> void visit(EntityConditionList<T> list); void visit(EntityFieldMap fieldMap); void visit(EntityConditionFunction func, EntityCondition nested); <T extends Comparable<?>> void visit(EntityFunction<T> func); void visit(EntityWhereString condition); void visit(EntityDateFilterCondition condition); } {code} I am still puzzled by the presence of {{visit}} methods with multiple arguments, so I will work with [~gil portenseigne] on understanding the functional logic of the {{EntityCondition}} class hierarchy to check if that weirdness make sense. was (Author: mthl): I have uploaded [^OFBIZ-10593_Implement-EntityConditionVisitor-properly.patch] which is a WIP patch trying to improve the situation the following {{EntityConditionVisitor}} interface: {code:java} public interface EntityConditionVisitor { <T extends EntityCondition> void visit(EntityJoinOperator op, List<T> conditions); <L extends EntityCondition, R extends EntityCondition> void visit(EntityJoinOperator op, L lhs, R rhs); <L,R,T> void visit(EntityOperator<L, R, T> op, L lhs, R rhs); <L,R> void visit(EntityComparisonOperator<L, R> op, L lhs, R rhs); void visit(EntityConditionValue.ConstantNumberValue value); void visit(EntityFieldValue value); void visit(EntityExpr expr); <T extends EntityCondition> void visit(EntityConditionList<T> list); void visit(EntityFieldMap fieldMap); void visit(EntityConditionFunction func, EntityCondition nested); <T extends Comparable<?>> void visit(EntityFunction<T> func); void visit(EntityWhereString condition); void visit(EntityDateFilterCondition condition); } {code} I am still puzzled by the presence of {{visit}} methods with multiple arguments, so I will work with [~gil portenseigne] on understanding the functional logic of the {{EntityCondition}} class hierarchy to check if that weirdness make sense. > ‘EntityConditionVisitor’ is a confused visitor pattern > ------------------------------------------------------ > > Key: OFBIZ-10593 > URL: https://issues.apache.org/jira/browse/OFBIZ-10593 > Project: OFBiz > Issue Type: Improvement > Affects Versions: Trunk > Reporter: Mathieu Lirzin > Assignee: Mathieu Lirzin > Priority: Major > Attachments: > OFBIZ-10593_Implement-EntityConditionVisitor-properly.patch > > > {{EntityConditionVisitor}} interface is supposed to implement the visitor > pattern which supposes a set of classes meant to be visited using an > {{accept}} method and a visitor with multiple {{visit}} method overloads (one > for each visited class). > Currently {{EntityConditionVisitor}} contains both {{accept}} and {{visit}} > methods which make *no sense* > {code:java} > public interface EntityConditionVisitor { > <T> void visit(T obj); > <T> void accept(T obj); > void acceptObject(Object obj); > void acceptEntityCondition(EntityCondition condition); > <T extends EntityCondition> void > acceptEntityJoinOperator(EntityJoinOperator op, List<T> conditions); > <L,R,T> void acceptEntityOperator(EntityOperator<L, R, T> op, L lhs, R > rhs); > <L,R> void acceptEntityComparisonOperator(EntityComparisonOperator<L, R> > op, L lhs, R rhs); > void acceptEntityConditionValue(EntityConditionValue value); > void acceptEntityFieldValue(EntityFieldValue value); > void acceptEntityExpr(EntityExpr expr); > <T extends EntityCondition> void > acceptEntityConditionList(EntityConditionList<T> list); > void acceptEntityFieldMap(EntityFieldMap fieldMap); > void acceptEntityConditionFunction(EntityConditionFunction func, > EntityCondition nested); > <T extends Comparable<?>> void acceptEntityFunction(EntityFunction<T> > func); > void acceptEntityWhereString(EntityWhereString condition); > void acceptEntityDateFilterCondition(EntityDateFilterCondition condition); > } > {code} > this confusion is visible in the {{EntityCondition}} which has both a > {{visit}} and an {{accept}} method, even if it is only supposed to accept a > visitor. -- This message was sent by Atlassian JIRA (v7.6.3#76005)