retrosheet icon indicating copy to clipboard operation
retrosheet copied to clipboard

Improve `isReturnTypeList` for non suspend method

Open tuanchauict opened this issue 4 years ago • 2 comments
trafficstars

Currently, as mentioned in #11 , isReturnTypeList can work with any return type which contains java.util.List. However, this approach has some issues:

  1. Incorrect if the return type is something like this: Foo<Bar<Baz<List<Something>>>
  2. Cannot work with other collection types like Set, Array.

I tried an update like this:

private fun isMethodWithListReturnValue(method: Method): Boolean {
    if (method.returnType.isArray) {
        return true
    }
    val genericReturnType = method.genericReturnType as? ParameterizedType ?: return false
    return isArrayOrCollectionClass(genericReturnType.rawType.typeName)
}

However, this doesn't work if List-type is the secondary type like: Flow<List<Something>>.

If we extract secondary type from Flow like what we did with Continuation for suspend method, we cannot guarantee whether the list is hidden under the third type like Flow<Resource<List<Type>>>.

tuanchauict avatar May 03 '21 09:05 tuanchauict

Another approach is to stop supporting too many types. We can limit to only suspend or normal function which returns List or Collection.

IMO, if a dev wants to support Flow for API, he/she can easily create a repository class. The API just does request and response data, not handle the other things (Single responsibility principle).

I definitely vote for this. @theapache64 What do you think?

tuanchauict avatar May 03 '21 09:05 tuanchauict

@tuanchauict

Sorry for the late reply. I somehow missed this issue.

Incorrect if the return type is something like this: Foo<Bar<Baz<List<Something>>>

As of now, technically, retrosheet can only return an Object or List<Object>. so the above given generic wouldn't be technically possible at all. This means, even if we write support for that, that case won't occur.

If we extract secondary type from Flow like what ...

If the consumer wants to use Flow, Resource, or any other custom type. They'll have to write custom retrofit call adapters for that. I think this doesn't come under retrosheet.

theapache64 avatar Jul 24 '21 15:07 theapache64