search

steamclock / netable

A Swift library for encapsulating network APIs using Codable in a type-oriented way.
MIT License
99 stars 3 forks source link

add combine support for requests #89

Closed brendanlensink closed 3 years ago

brendanlensink commented 3 years ago

For #64

This adds a pretty simple second request function that returns a PassthroughSubject<FinalResource, NetableError>.

I had considered returning PassthroughSubject<Result<FinalResource, NetableError>, Never>(), since it more closely matches our other request function and leverages Result, but I think it makes more sense to match Combine. Is that reasonable?

nbrooke commented 3 years ago

I think in this case haven it just be PassthroughSubject<FinalResource, Error> make sense.

You could imagine some weird situations where the "value" of a Combine publisher also including an error might make sense, because errors implicitly terminate publishers, any case where you could logically send multiple errors would need to be represented like that. Say a interface with some retrying where it distinguishes "here's an error I got retrying, could still send another error or a success" vs. "I've stopped retrying and failed permanently". However, I don't think THIS interface is one of those, so I think just doing it the logical and simple way makes sense.

One thing we will need to think about and keep an eye on with this interface once we start using it is if it makes sense to have a different name, functions overloaded by return type are always a little creepy, and if we ended up having to add in a bunch of api.request(/*...*/) as Result<Foo, NetableError> to disambiguate, that'd be pretty bad. One thing we should try before we 100% commit to this is making sure that if we pull this version into an app that uses Netable and that there are no ambiguities created in existing code by having the two overloads.