Moya encapsulates everything necessary to make a call, but on the return requires clients to parse and cast Result<Moya.Response, MoyaError> objects, which inevitably clutters the completion block and leaks network concerns into the client. From #948 as @scottrhoyt characterized (without endorsing) it, the goal is "Provide better completion handler usability for MoyaProvider when both the return type and method of deserialization are known".
This can be accomplished adding the following (queue, progress parameters removed for clarity) generic function to MoyaProvider:
/// Generic request-making method.
@discardableResult
open func request<T>(_ target: Target, parser: @escaping (Moya.Response) throws -> T, completion: @escaping (Result<T, MoyaError>) -> Void) -> Cancellable {
return request(target, queue: queue, progress: progress) { (result) in
do {
let response = try result.dematerialize()
let value = try parser(response)
completion(Result(value: value))
} catch let error as MoyaError {
completion(Result(error: error))
} catch {
completion(Result(error: MoyaError.underlying(error)))
}
}
}
It's straightforward, it calls the parser on the response and returns the value (or any error) to the completion. The generic function enforces that the type returned by the parser matches the parameter of the completion. (Side note, this is a great example of why to avoid optional sentinels: T can be an optional where nil is valid value, so only throws indicates an error.)
So the call site becomes:
func downloadRepositories(_ username: String) {
GitHubProvider.request(.userRepositories(username), parser: Parsers.parseNSArray) { result in
switch result {
case let .success(json):
self.repos = json
self.tableView.reloadData()
case let .failure(error):
guard let error = error as? CustomStringConvertible else {
break
}
self.showAlert("GitHub Fetch", message: error.description)
}
}
}
It may seem like a small distinction, moving the parser from something invoked in the completion to something passed to the request method; the call site still needs to know how the response will be parsed. The main benefit is it declutters the completion blocks, removing any knowledge or handling of response objects. A similar approach would work with the reactive versions. It also seems more in line with the goals of encapsulation to handle these things upstream.
Heads-up in the meantime - you can highlight the syntax in the code snippets with ```swift at the start. :)
Interested to hear what other @Moya/contributors think about this.
I would still lean towards saying that unless Moya gets in the game of deserializing objects, this is the purview of Moya Extentsions. This could be added via an extension on MoyaProvider. However, an approach like this does have the advantage of creating a common interface for extension providers, and, thus, allows them to standardize their own interfaces.
I think one reason this hasn't been a bigger issue is that I think a large percentage of the user base is using the reactive providers, which already provide a cleaner interface for response handling and object deserialization through Moya extensions. There's also probably some legacy reasons since the current callback pattern aligns closely with Alamofire's.
I still don't have a position on this matter.
My first thought was to agree with @scottrhoyt:
I would still lean towards saying that unless Moya gets in the game of deserializing objects, this is the purview of Moya Extentsions.
Extended support for parsing feels a bit outside the scope of Moya, which is nicely pointed in the Readme:
So the basic idea of Moya is that we want some network abstraction layer that sufficiently encapsulates actually calling Alamofire directly.
On the other hand, if you check all plugins listed in the Readme, only MoyaSugar is not about JSON serialization:
Moya-ObjectMapper - ObjectMapper bindings for Moya for easier JSON serialization
Moya-SwiftyJSONMapper - SwiftyJSON bindings for Moya for easier JSON serialization
Moya-Argo - Argo bindings for Moya for easier JSON serialization
Moya-ModelMapper - ModelMapper bindings for Moya for easier JSON serialization
Moya-Gloss - Gloss bindings for Moya for easier JSON serialization
Moya-JASON - JASON bindings for Moya for easier JSON serialization
Moya-JASONMapper - JASON bindings for Moya for easier JSON serialization
Moya-Unbox - Unbox bindings for Moya for easier JSON serialization
MoyaSugar – Syntactic sugar for Moya
Moya-EVReflection - EVReflection bindings for Moya for easier JSON serialization (including subspecs for RxSwift and ReactiveCocoa)
Moya-Marshal - Marshal bindings for Moya for easier JSON serialization
Is this an indication that we should be giving better support for JSON serialization? Why does everyone feel like they have to build their own?
This can be seen as a missing feature, but also as flexibility: parsing can be done in several ways and you can pick the one that fits your app better.
A while ago I was trying to make something similar (make an option to TargetType that can map the response), but given tools of Swift 2.2 I wasn't satisfied with any of my results.
Unfortunately with your proposition and passing parser to request function it makes _me_ responsible for remembering how should I parse the response every time, so I still need some abstraction layer over Moya to get what I want (thus I don't see much pros in this one). Although as @scottrhoyt said, this have an advantage with option to make a standard protocol for object parsing.
@pedrovereza
I think the quantity of JSON parsing extensions is just representative of how many popular JSON deserialization libraries there are. That is indicative of how for a while no single best practice had emerged in crossing the weakly typed/strongly typed gap. It seems now that a single pattern has emerged in multiple libraries.
Actually, what I would like to see is a Swiftier implementation of NSCoding that all these libraries could leverage or perhaps interface directly with JSONSerialization. It would be great to have better basic language support for object serialization that cares at least as much about JSON as PLIST.
As for better support of object serialization in Moya, that would mean either:
It's hard for me to argue for 1 when we haven't written our own Alamofire. And doing 2 would come with the responsibility of recommending a best choice of library(s). I don't know if there is one right now. The last time I made this choice, I just picked the one that was already in my dependency graph.
As an aside, it is interesting that the highest level of abstraction in this stack is really the Moya extension you choose for JSON deserialization. Drop that in your Cartfile/Podspec and you get everything else you need.
The idea is to take Moya the last mile, so clients don't have to do parsing in their completion blocks at the call site. I think @pedrovereza 's comment that's what nearly all the extensions do reflects that. In this generic solution, since there's no way through Swift's type system to have enums vend closures returning different types, the call site still needs to know which parser to use. Putting the parser in the function signature removes one concern from the completion.
(This is obviously less of a concern for the reactive variations that use lenses throughout their entire structure. Of course it could be moved up for them as well.)
As @scottrhoyt said allowing extensions to standardize their own interfaces would be a benefit and allow better integration. Because of generics this needs to be done carefully, as the generic function parameters func request(TargetType, (Response) -> T, completion (T) -> Void)don't hold up when passed a generic function--legitimately, because all type information is gone. Typing the completion block only sometimes fixes this.
Actually, browsing through the extensions it seems like all of them work by requiring T to conform to some protocol -able for the library, usually init(_ json: JSON)? Assuming the compiler could be instructed what the type is, it would be possible to have a protocol init(from response: Moya.Response) throws and put the requirement in the generic function constraint.
However, maybe it's my experience parsing terrible JSON but I would be against creating the expectation that JSON parsing can be accomplished automatically from endpoint-magic-object. There's a gap between endpointJSON and model, that clients need to be explicitly and consciously bridge. The place to do that is the TargetType, but that was a dead end for enums.
In the case of completion blocks it makes sense that the knowledge of how to get the response into the app domain should have been made as part of the call. Baking _the option_ to provide a parser of choice -- an option, because this would be able to live alongside the existing method and immediately use into it -- would make Moya more end-to-end by covering the final distance.
My thoughts: https://xkcd.com/927/
On Wed, Feb 8, 2017 at 9:39 AM Michael Sanderson notifications@github.com
wrote:
The idea is to take Moya the last mile, so clients don't have to do
parsing in their completion blocks at the call site. I think @pedrovereza
https://github.com/pedrovereza 's comment that's what nearly all the
extensions do reflects that. In this generic solution, since there's no way
through Swift's type system to have enums vend closures returning different
types, the call site still needs to know which parser to use. Putting the
parser in the function signature removes one concern from the completion.(This is obviously less of a concern for the reactive variations that use
lenses throughout their entire structure. Of course it could be moved up
for them as well.)As @scottrhoyt https://github.com/scottrhoyt said allowing extensions
to standardize their own interfaces would be a benefit and allow better
integration. Because of generics this needs to be done carefully, as the
generic function parameters func request(TargetType, (Response) -> T,
completion (T) -> Void)don't hold up when passed a generic
function--legitimately, because all type information is gone. Typing the
completion block only sometimes fixes this.Actually, browsing through the extensions it seems like all of them work
by requiring T to conform to some protocol -able for the library, usually init(_
json: JSON)? Assuming the compiler could be instructed what the type is,
it would be possible to have a protocol init(from response:
Moya.Response) throws and put the requirement in the generic function
constraint.However, maybe it's my experience parsing terrible JSON but I would be
against creating the expectation that JSON parsing can be accomplished
automatically from endpoint-magic-object. There's a gap between
endpointJSON and model, that clients need to be explicitly and consciously
bridge. The place to do that is the TargetType, but that was a dead end for
enums.In the case of completion blocks it makes sense that the knowledge of how
to get the response into the app domain should have been made as part of
the call. Baking the option to provide a parser of choice -- an option,
because this would be able to live alongside the existing method and
immediately use into it -- would make Moya more end-to-end by covering the
final distance.—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Moya/Moya/issues/950#issuecomment-278202738, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADo1dCOtObOkVw2WqdCSy7OdpQJyvrHmks5raRzJgaJpZM4L2xjp
.
You know, I'm going to follow the "Contributing" guidelines in the readme.md, assume positive intent, and give @AndrewSB as well as the people who reacted with "laugh" the benefit of the doubt. I love XKCD, and assume people are taking the moment to laugh at this classic.
But I would invite you to think about how it could come across to post that comic in this context. I've put a lot of thought and a lot of time on this proposal because I think it would improve Moya, a project I've watched for a long time now, that I think is ingenious and greatly respect.
I'm happy to continue to respond to any substantive comments about the idea of adding the MoyaProvider generic function.
I love XKCD, and assume people are taking the moment to laugh at this classic.
@michaelsand Yes, I was honestly laughing at the comic itself, no intention to make fun of your proposal. Apologies if it sounded that way.
I was just happy that I wasn't the only one laughing about the state of JSON deserialization in Swift! :) Your ideas are truly welcome @michaelsand. This is what pushes the project forward.
Sure, comic about a guy making a proposal was just an off note to drop in this context, no big deal.
While the generic function is being mulled, since I did browse through the extensions I noticed that no one is using the Result method dematerialize() throws -> T. The switch statement usually makes more sense, but when the success case immediately starts a do-try-catch block it could be much cleaner.
I'm looking at this also in the example in the Demo ViewController func downloadRepositories(_ username: String) here. Reacting to this in particular was part of what I was trying to improve with this and the earlier #948 proposal.
It current looks like:
func downloadRepositories(_ username: String) {
GitHubProvider.request(.userRepositories(username)) { result in
switch result {
case let .success(response):
do {
if let json = try response.mapJSON() as? NSArray {
// Presumably, you'd parse the JSON into a model object. This is just a demo, so we'll keep it as-is.
self.repos = json
} else {
self.showAlert("GitHub Fetch", message: "Unable to fetch from GitHub")
}
} catch {
self.showAlert("GitHub Fetch", message: "Unable to fetch from GitHub")
}
self.tableView.reloadData()
case let .failure(error):
guard let error = error as? CustomStringConvertible else {
break
}
self.showAlert("GitHub Fetch", message: error.description)
}
}
}
With dematerialize() it can come to:
func downloadRepositories(_ username: String) {
GitHubProvider.request(.userRepositories(username)) { result in
do {
let response = try result.dematerialize()
let value = try response.mapNSArray()
self.repos = value
self.tableView.reloadData()
} catch {
let printableError = error as? CustomStringConvertible
let errorMessage = printableError?.description ?? "Unable to fetch from GitHub"
self.showAlert("GitHub Fetch", message: errorMessage)
}
}
}
As nice as do-try-catch setup as I've seen (not an end in itself, but still) and the smooth handling of error message with fallback it makes possible is a nice bonus.
But the mapNSArray() function as an extension of Response doesn't currently exist. There could be a guard statement in the do block, but you would have to throw from the do block, and now what error type do you use, etc.
I'm thinking it should be in the Demo GitHub file. All it does is cast the response from mapping to Any. (Although it's just file organization at this point, as I mentioned in #948 and above I think the knowledge of how to parse the response is deeply tied to the endpoint/TargetType anyway.) Another option would be to name the function mapGitHubRepos() to emphasize the view controller is passing all responsibility for how to parse this endpoint, and also because there are different ways to parse to the same type.
It would also accomplish much of what I was trying to without touching the core project, so could shelve the generic function for now until there are reasons to do the parsing before returning result to the completion. If the information about what parser to use has to live at the call site anyway, getting it done in one line this way is just as compact.
Thoughts? (words preferred ;)
@michaelsand I'm sorry about how l made you feel by posting a comic. I had no intention of disrespecting you, or the proposal you clearly have put a lot of thought into.
I'm not working right now, so I haven't been spending as much time keeping up with the project, when I glanced at your issue in an email it was the first thing my mind jumped to.
I should have provided substantive feedback, and put greater thought into how simply posting a comic would have come across in this situation.
I hope I haven't tainted your opinion of this community and that you're still excited to contribute to this project.
I'm still 100% against Moya taking an opinionated stance on how to handle responses. It's easy for us to extend the Response type with more properties from the NSURLSession or even the NSHTTPResponse, but it gets incredibly nasty when we start saying "this is how you really wanted the data to be" and start adding all sorts of JSON/XML/(insert response format here) parsers and types for handling those. The worst thing we can do, by providing a native parser, is force it on our users, thus removing the flexibility of the library and introducing some wild wild west for working around such an opinion.
As @sunshinejr mentioned, there's still going to be some abstraction implemented. With the reactive providers, you can easily extend the reactive types to process the response, and I think that consumers of the default MoyaProvider should be doing the something similar.
One implementation suggestion is to provide serialization closures, which get passed to the MoyaProvider:
func zenResponseHandler(result: Result<Moya.Response, Moya.Error>) {
/// Handle the zen response
}
provider.request(.zen, completion: zenResponseHandler)
Another suggestion would be to extend the MoyaProvider and implement your own request methods:
extension MoyaProvider {
func requestZen() -> Result<MyResponse, MyError> {
return provider.request(.zen) { result in
/// Process the response
}
}
}
I completely understand the motivation, but what I've found in the multiple projects and applications I've built that use Moya is that there is no single one-size-fits-all solution. Never have I pulled a serialization layer from a previous project and used it in a future project.
EDIT:
As OP mentioned, wouldn't it be possible to just extend the Response type to provide custom mapping functions to transform to custom types?
extension Response {
func mapObjects<T>() -> throws [T] {
guard let myResponseType = MyType(data) else { throw .mapError }
return myResponseType
}
}
@justinmakaila What you said here really resonates with my own opinion (that Moya should not get in the response deserialization game), but I also don't exactly agree with what you said. What @michaelsand is proposing here is not a parser library or system, just another take on how to integrate a parsing layer at the call site.
This is usually the type of feature that I don't like adding, but in this case the more I look at it the more I like it!
It might be unfortunate that the title includes "method of deserialization is known" because that makes it sound like Moya is getting involved, but actually it's just saying that you could plug any parser into this; the parsers (that you write as part of your network layer) take a Moya.Response (if the result is .success) and hand the result (or a new error) to your completion block.
The reason I like this proposal is that it adds the type information - and only the type information - to the call site, and the type is inferred by the compiler based on the mapping function, so there's no additional noise. And mapping from Result<Moya.Response> to Result<T> feels very natural to me. Put another way, it is a way to inform Moya how it can get out of the way of our application. We begin the request cycle by handing it an enum value, we end by receiving our expected object or an error.
And we get "opt-in/opt-out" for free here since this method signature would not collide with the existing request method. In the Ello app we look at the HTTP result directly to get the status code out, so we could still use this method to map to a (statusCode, JSON) tuple, and I think our code would benefit from it! Since our JSON is in a consistent format, we could actually parse it into a really useful object, which would move a lot of the heavy lifting out of ElloProvider and into a testable/pluggable ElloJSONParser type.
I'm on board with this change. It is backwards compatible, it feels very "functional programming"-esque, and I don't see it as opinionated as @justinmakaila seems to. To me, this is a natural place to provide an HTTP response parser.
I would love to see a dissenting opinion that shows how we can get this same benefit from our current setup. Thanks!
FWIW, my perspective is that of a ReactiveMoya user. I guess the way I'm viewing Moya is that it takes in a TargetType and it gets you a Response. I get the response, check it for errors, map it into some result and send it on it's way.
Taking on @colinta's challenge for a dissenting opinion:
public func authenticate(type: AuthType, username: String, password: String) {
provider.request(.auth(type, username: username, password: password), completion: { result in
do {
let response = try result.dematerialize()
let authToken = try response.mapAuthenticationResponse()
/// Store the auth token
/// Update the UI
} catch let error as AuthenticateError {
/// Show an alert
self.showAlert(title: error.title, message: error.message)
} catch {
/// Handle other errors
}
})
}
extension Moya.Response {
func mapSwiftyJSON() throws -> JSON {
let json = try mapJSON()
return JSON(json)
}
func mapAPIResponse() throws -> MyAPIResponse {
let json = try mapSwiftyJSON()
return MyAPIResponse(json: json)
}
func mapAuthenticationResponse() throws -> String {
do {
let response = try mapAPIResponse()
} catch {
throw AuthenticateError.underlying(error)
}
if response.isError {
/// Parse and throw an Authenticate Error
throw AuthenticateError.invalidPassword("Password is invalid")
}
if let authToken = response.data["authToken"] {
return authToken
}
throw AuthenticateError.invalidToken("Could not find valid auth token")
}
}
enum AuthenticateError {
case invalidToken(String)
case invalidUsername(String)
case invalidPassword(String)
case underlying(MoyaError)
var title: String {
return "Uh-oh"
}
var message: String {
switch self {
case let .invalidPassword(message):
return message
case let .invalidUsername(message):
return message
}
}
}
The more I look into this, the more I realize that this is just a recreation of what the reactive users already have (in terms of this).
I think this is mostly an improvement for the users who don't use any of the extensions. I like the opt-in/opt-out functionality, and the non-breaking aspect of it. I guess my main point is that this functionality is currently achievable by extending the Response, though it's not immediately obvious.
Edit
Updated to reflect concerns from @michaelsand regarding throwing errors of certain types... I really dislike the concept of throwing in Swift because, in most cases, you have to have domain information to know what kind of Error the function is going to throw, given the fact that the Error type isn't dictated in the method signature.
Great counter point! Extending Moya.Response achieves similar code separation; it isn't as purely functional perhaps, but that's splitting hairs. I would be hard pressed to call one more "Swifty". map(a) -> b is insteada.map() -> b.
And, unless I'm mistaken, @michaelsand might also be agreeing with this strategy; I was re-reading the conversation here, and in his last comment he points out that extending Moya.Response accomplishes the same goal (I missed that the first time around).
On the other hand, I had a different use-case in mind. In our app, we have only one place where we call Moya, and that's from ElloProvider. We do authentication token checking and refreshing before we send the request out. So in our case, the "parser" that I would provide is higher-level; it would only return a generic SwiftyJSON object (and it would introspect that JSON - we use a linked key and put all the related objects in there into local key-value storage - typical JSON API pattern). In your example, this corresponds with let response = try mapAPIResponse(), and so the response body would look something like this:
public func authenticate(type: AuthType, username: String, password: String) {
let endpoint: API = .auth(type, username: username, password: password)
provider.request(endpoint, parser: mapAPIResponse, completion: { result in
do {
let apiResponse = try result.dematerialize()
let authResponse = try mapAuthenticationResponse(apiResponse) # MyAPIResponse instance
}
...
I do still like this; it "closes the loop" between your domain's API endpoints, and how your data is returned from the server. But is it necessary? No, I think @justinmakaila demonstrated that we can accomplish this by having the parser as part of the body; and either way (whether it's in the function, or in the body) you'd have to repeat this parser code as many times as you have Moya call sites.
We've bumped up against the limit of what is possible with generics in Swift. The compiler needs type information sooner or later. IMO the safest and DRYest way to do that is to wrap the provider in an object that provides type information and deserialization. I know it doesn't feel as nice as we'd like it to, but neither does the unconstrained polymorphism of the other options or creating one TargetType per return type. The above options can help you write that wrapper, but I would say a better separation of concerns is to put the extension on the response deserializer itself by allowing it to ingest Data and then pass it Response.data. Alternatively you could just use the output from mapJSON, a dictionary that nearly all the libraries support.
As an aside, I assumed that the reason we don't see dematerialize used more often was that we were using Alamofire.Result and not Result.Result. I see I am wrong. We should probably add that to our Cartfile explicitly instead of relying on it being there (from ReactiveSwift, I think). I'll open up a PR for that.
@AndrewSB no big deal, thanks for mentioning that.
@colinta
I really like the formulation "it is a way to inform Moya how it can get out of the way of our application."
@justinmakaila Somewhat out of order:
FWIW, my perspective is that of a ReactiveMoya user.
Yes, it is apparent Reactive/RX doesn't get as much out of this, since if going from A to Z and Moya gets you from A to Y, it would make sense immediately apply a lens for Y to Z--in a functional reactive app it just seems more natural. It's the traditional completion block approach (I'm going to keep calling them completion blocks) that makes having to do parsing at the call site a violation of separation of concerns, and currently users are on their own, why it's noted there are so many extensions for it. Of course if parity is needed it's completely possible.
One implementation suggestion is to provide serialization closures,
As far as providing Moya a function that takes a response that Moya then invokes, if that closure still needs to know about parsing logic and about view-controller completion logic, it's just moving code around. There is a way around this, in the (brief) #948 I outlined a technique with enum that accomplishes what I set out to do, let the TargetType switch to pick the parser, but at a cost that the completion closure must be added as an associated type. It puts this complex requirements on users and the syntax looks bad also.
As OP mentioned, wouldn't it be possible to just extend the Response type to provide custom mapping functions to transform to custom types?
Taking on @colinta's challenge for a dissenting opinion
This is an even fully-formed than my example. If this is the way we land, we should definitely put a more elegant demo in there, with a parser defined in the demo project, probably in GitHub.swift.
I have reservations about telling people this much in an extension. There's that MyAPIResponse class tucked in there. AuthenticateError is free-floating. It calls other functions in the extension. And actually my expectation is the parser could have some settings, like debugging options. Of course a response extension could have/refer those as well (today I learned extensions can add static var but if it's a static var might as well put it on some other static settings object.)
Still, moving the parser to a parameter would allow the block to swich on result if it wanted to. It would emphasize the parsing is something that is not this class's responsibility, emphasizing that Moya can handle it end-to-end.
@scottrhoyt Comment on your aside regarding Result: We need to be mindful about the version numbers. Since ReactiveSwift also relies on it, we need to keep that in sync with their version, especially for the users that are using the binary distribution (i.e. me).
@michaelsand I agree with a lot of what you said, especially around the free-floating AuthenticateError. However, I don't think calling the other functions is a deal, particularly because the Error stays as a MoyaError until you hit the mapAPIResponse().
As I mentioned, it's not the prettiest in regards to error handling, but that could easily be changed to work like so (this is untested sudo-code, my brain compiler gives it the 👍):
extension Response {
func mapAuthenticationResult() -> Result<String, AuthenticateError> {
do {
let authToken = try mapAuthenticationResponse()
return .success(authToken)
} catch error as AuthenticateError {
return .failure(error)
} catch {
return .failure(.underlying(error))
}
}
}
IMO, the most valuable part of the parser function is the type information that can be provided.
That said, since we would be changing the call site to pull the correct parser and apply it, I'm leaning with @scottrhoyt that we should just wrap the provider and modify it to work accordingly with completion blocks.
Wrapped Example:
struct NetworkProvider {
struct Parsers {
static func authentication(type: AuthType) -> ((Moya.Response) -> String) {
return { response in
/// Transform response
}
}
}
var provider: MoyaProvider<MyAPI>
func request(target: MyAPI, parser: (Moya.Response) -> T, completion: Result<T, MoyaError>) {
provider.request(target) { result in
do {
let response = try result.dematerialize()
completion(.success(parser(response)))
} catch {
completion(.failure(.underlying(error)))
}
}
}
}
let provider = /// Create some provider
let networkProvider = NetworkProvider(provider)
networkProvider.request(.auth(.signIn, username, password), NetworkProvider.Parsers.authentication(.signIn)) { result in
/// TODO: Handle the result
}
In-line Example:
public func authenticate(type: AuthType, username: String, password: String) {
let parser = Parsers.authentication(type)
provider.request(.auth(type, username: username, password: password), completion: { result in
do {
let response = try result.dematerialize()
let authToken = parser(response)
/// Store the auth token
/// Update the UI
} catch let error as AuthenticateError {
/// Show an alert
self.showAlert(title: error.title, message: error.message)
} catch {
/// Handle other errors
}
})
}
@colinta Yeah I'm in the awkward position of not being sold on the necessity of my own proposal, since well-formed extensions on Response get the job done. Regardless some of the Demo examples could be a lot cleaner, especially downloadRepositories
I should mention my reason for being enthusiastic about this: I implemented this generic pass-parser approach for handling completions and it was a lifesaver. This was when I worked at an agency, a client was gonna be the next facebook so the app had a lot of API calls. This was before Moya existed, and I had each call made in its own function, a network wrapper. But on the completion, all of the network calls went through one generic completion block, which really was a lifesaver when their dysfunctional API kept breaking and we needed to start logging/scrutinizing everything.
Anyway, after that when Moya I saw that this handled most of the other steps more elegantly, and with the addition of generic completion blocks could wrap the entire networking layer end-to-end.
But yeah, there are other ways to accomplish that goal. So it feels like a glaring omission to drop the raw response into the call site, probably a view controller--but generics are trouble, and this is coming down to really accomplishing that much.
@scottrhoyt Yeah limitations generics in Swift is the way to describe it. I ended up going down the rabbit hole of covariance. Not fun. The rabbit can't even live in a rabbit hole-- it has to live in a generic burrow.
@justinmakaila That wrapped provider looks good for another layer of safety. I feel like the integration with TargetType is so close, and was looking at solutions with hashable identifier; will keep investigating.
I'm going to go ahead and submit the pull request to clean up those completion blocks in the Demo, adding a extension Response for mapping to NSArray. If this is recognized as a best practice might as well put it in a demo.
I'm also going to put some of the various loose threads into a pod for an extension, including some extensions on MoyaProvider that might be useful depending on what trade-offs best meet various use-cases.
Thanks for the great feedback everyone-- not the end, but still.
Thank you @michaelsand for the thoughtful idea and discussion. I personally would love to have you contribute to Moya wherever/whenever you'd like!
I'm gonna close this one for now since there is no activity, but feel free to reopen it whenever.
Most helpful comment
You know, I'm going to follow the "Contributing" guidelines in the readme.md, assume positive intent, and give @AndrewSB as well as the people who reacted with "laugh" the benefit of the doubt. I love XKCD, and assume people are taking the moment to laugh at this classic.
But I would invite you to think about how it could come across to post that comic in this context. I've put a lot of thought and a lot of time on this proposal because I think it would improve Moya, a project I've watched for a long time now, that I think is ingenious and greatly respect.
I'm happy to continue to respond to any substantive comments about the idea of adding the MoyaProvider generic function.