-
Notifications
You must be signed in to change notification settings - Fork 38
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
Merge extra commands having different returning container types #44
Comments
The problem is that we should always use A solution is to use implicit conversion from object ZRangeReturn {
implicit def apply[A: Render](withScores: Boolean): ZrangeReturn =
if (withScores) WithScores[A]() else Default[A]()
} |
And |
How about returning an |
I'm not sure where to put it. Would you give me an insight? |
oops .. I meant |
I see. It will let user know the return type more explicltly. IMHO, however, it would require additional steps from both of us (having another special deserializer) and users (should work on projection). Can it be sufficient with proper API documentation about the return type? |
Currently I think proper API documentation for the return type is fine. Just wanted to raise it as a question to see what others feel about it. Yes, we need a de-serializer but that should not be too difficult. But how do you feel about Either being the return type. Does it make an API user feel that some things are being forcefully grouped together ? Or we can have the |
Agree on your point. As these are tradeoffs and it's not urgent, let's just keep it open for now and see if there's anyone interested in. |
The current solution has the advantage of having a specific return type at both the command and operation level. It sounds like merging the case classes would mean adding another layer of complexity to the return type processing, which I'm not particularly fond of doing. Also, I generally use Either as a return type when I expect the left hand side to contain any errors if the command failed. |
@andyscott +1 on your thoughts on simplicity of the current implementation. The alternative was an approach towards DRY. Regarding So as @guersam suggested let's keep it open for the time being. |
Currently we have extra case classes like
ZRangeWithScores
to support for commands with varying return types according to it's argument.We might merge them into original commands with path-dependent type like this:
I'll give it a try when time allows.
The text was updated successfully, but these errors were encountered: