[ 
https://issues.apache.org/jira/browse/GEODE-1577?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15839031#comment-15839031
 ] 

ASF GitHub Bot commented on GEODE-1577:
---------------------------------------

Github user upthewaterspout commented on the issue:

    https://github.com/apache/geode/pull/321
  
    Hi all,
    
    I'm really sorry I missed this. I was out of town for a while and somehow 
the discussion slipped through the cracks when I was catching up.
    
    These changes are what I had in mind, but I didn't realize that this would 
break existing code that was doing a cast. I know adding a generics should be 
safe, but in this case we're fixing generics that are broken which is why I 
think we are getting into trouble.
    
    I did a little messing around. It looks like we might be able to get away 
with adding generics to Execution. 
    The only caveat is that once we add generics to Execution we are stuck with 
them. There is an existing bug to also add an argument type to the 
FunctionContext - see GEODE-2217. So I think maybe the thing to do is to add 
three types parameters to execution. Something like this:
    
    ```
    /**
     *
     * @param <IN> The type of the argument passed into the function, if any
     * @param <OUT The type of results sent by the function
     * @param <AGG> The type of the aggregrated result returned by the 
ResultCollector
     */
    public interface Execution<IN, OUT,AGG> {
      Execution<IN, OUT, AGG> withArgs(IN args);
      ResultCollector<OUT, AGG> execute(...)
    }
    ```
    
    What do you think? The other option I suppose is to deprecate the Execution 
class and introduce a new one, or deprecate the execute methods and introduce 
new ones, but that seems painful.


> Unhelpful generic types on Execution.execute
> --------------------------------------------
>
>                 Key: GEODE-1577
>                 URL: https://issues.apache.org/jira/browse/GEODE-1577
>             Project: Geode
>          Issue Type: Bug
>          Components: functions
>            Reporter: Dan Smith
>            Assignee: Dan Smith
>              Labels: starter
>
> The execute methods of the function service Execution class returns a 
> ResultCollector with wildcards for the type.
> {code}  
> public ResultCollector<?, ?> execute(
>       Function function) throws FunctionException;
> {code}
> Wildcards are supposed to be used in APIs where the type doesn't matter, for 
> example counting the elements in a list. By returning a ResultCollector with 
> wildcards, we're essentially forcing the user to cast the result collector.
> At a minimum they should be able to pick the type of result collector
> {code}
>   public <T,S> ResultCollector<T, S> execute(
>       Function function) throws FunctionException;
> {code}
> But maybe it would make more sense to parameterize Execution itself. Then the 
> compiler could ensure that the types used by withCollector and the types used 
> by execute match.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to