gortiz commented on PR #10361:
URL: https://github.com/apache/pinot/pull/10361#issuecomment-1467570156

   > is there any reason why RegexpLike and FST-Based factories not part of the 
visitor pattern change?
   
   Just because I don't need it right now. I didn't iterate over all predicates 
to look for useful cases. In order to do not add features we may not use, I 
thought it would be better if we add them whenever we need them. But I can try 
to do it.
   
   > DataType visitor tests are not showing exactly how this will be useful yet.
   
   Given that DataType is an enum, we can use a switch and DataTypeVisitor is 
not that useful. I'm using it in a situation where the code in the switch 
depends on some context, so I do something like:
   
   ```java
   DataTypeVisitor<R> visitor;
   if (condition.is.fulfilled) {
     visitor = new VisitorInCase1;
   } else {
     visitor = new VisitorInCase2;
   }
   R result = dataType.accept(visitor)
   ```
   
   But I can use `Function` instead:
   
   ```java
   Function<DataType, R> fun;
   if (condition.is.fulfilled) {
     fun = dt -> {
       switch (dt) {
         case INT: return X1;
         case DOUBLE: return Y1;
         default: return Z1;
       }
     };
   } else {
     fun = dt -> {
       switch (dt) {
         case INT: return X2;
         case DOUBLE: return Y2;
         default: return Z2;
       }
     };
   }
   R result = fun.apply(dataType);
   ```
   
   The main difference is that the switch will not be protected by the compiler 
in case a new data type is added, but given that it isn't a very common change, 
I think I'm going to follow the second approach in order to create less 
confusion in this PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to