AsyncRequest return type should be known at compile time

[dnlbauer]

The current implementation of AsyncRequest#getDto() takes a generic parameter to specify the return type of the method. This makes it impossible to see what asyncRequest.getDto() is actually returning. It could be a summoner, a CurrentGameInfo or anything else. We only know it from looking at the context, but not from the object itself.

Additionally it can lead to runtime exceptions like this:

AsyncRequest req = riotApiAsync.getSummonersByName(Region.EUW, "danijoo");
CurrentGameInfo info = req.getDto(); // compiles fine, but throws a ClassCastException at Runtime

I wonder what led to this design decision. Wouldnt it be more convenient to have a fixed return type with an implementation like this?

class AsyncRequest<T> extends Request<T> {

     // ....

    @Override
    public T getDto() {  
        // ....
    }

}

AsyncRequest<Map<String, Summoner>> req = riotApiAsync.getSummonersByNames(Region.EUW, "danijoo");
CurrentGameInfo info = req.getDto(); // this wont compile because return type does not match.
Map<String, Summoner> summoners = req.getDto(); // this line works

Best Regards, Daniel

[Linnun]

Here are a few concerns:

How would that work when using listeners to wait for the result?

Example:

        // Asynchronously get summoner information
        AsyncRequest requestSummoner = apiAsync.getSummonersById(region, summonerId);
        requestSummoner.addListener(new RequestAdapter() {
            @Override
            public void onRequestSucceeded(AsyncRequest<Map<String, Summoner>> request) { // error because this is different from the overridden super method
                Map<String, Summoner> summoners = request.getDto();
                // ...
            }
        });

Sure, you could make RequestListener a RequestListener<T>, but then it could only listen to one specific method. Also nothing guarantees RequestListener<T> to have the same T as AsyncRequest<T>, which makes things very fishy.

Another thought, what's with the design pattern to have one listener listen to every asynchronous request via RiotApiAsync#addListener()? I have one use-case in production which actually implements RequestListener and listens to all kinds of requests. You could end up using a RequestListener<?> here, but that's even more fishy, since then you will have zero knowledge about the actual return type.

Also changing the RiotApiAsync methods this way would cause lots of conversion warnings which is dirty as hell:

    public AsyncRequest<Map<String, List<League>>> getLeagueEntryBySummoners(Region region, long... summonerIds) {
        Objects.requireNonNull(region);
        Objects.requireNonNull(summonerIds);
        return getLeagueEntryBySummoners(region, Convert.longToString(summonerIds)); // Type safety: The expression of type AsyncRequest needs unchecked conversion to conform to AsyncRequest<Map<String,List<League>>>
    }

[dnlbauer]

Yes, This would imply to change RequestListener to RequestListener<T>.

Also nothing guarantees RequestListener<T> to have the same T as AsyncRequest<T>, which makes things very fishy.

I dont think so. addListener(RequestListener> can be changed to addListener(RequestListener<T>). The addListener would only accept Listeners that can listen T then.

Also changing the RiotApiAsync methods this way would cause lots of conversion warnings

This is because type safty is not built in the other functions. If you start being typesafe from lowlevel on, they will disappear:

class EndPointmanager {
    public <T> AsyncRequest<T> callMethodAsynchronously(method) { .....}
}

ApiMethod<Map<String, Summoner> method = new GetSummonersByName(getConfig(), region, Convert.joinString(",", summonerNames));
return endpointManager.callMethodAsynchronously(method);

I know its a big change and would require a lot of code being changed but I think its worth it. The global listener is a problem though. I have to think about how this could be solved. hm..

[Linnun]

I recently had the idea to move towards a Future<V> design pattern for asynchronous requests.

This comment is mostly a reminder to myself to get back to it one day, but if anyone got any thoughts on this, please feel free to discuss and share your thoughts.