Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve isReturnTypeList for non suspend method #15

Open
tuanchauict opened this issue May 3, 2021 · 2 comments
Open

Improve isReturnTypeList for non suspend method #15

tuanchauict opened this issue May 3, 2021 · 2 comments

Comments

@tuanchauict
Copy link
Contributor

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
Copy link
Contributor Author

tuanchauict commented May 3, 2021

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?

@theapache64
Copy link
Owner

theapache64 commented Jul 24, 2021

@tuanchauict

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

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants