[WIP] Integrate with the Swift 4 Decodable Protocol

[ghost]

Just wanted to throw this out there early to get some thoughts from the maintainers of the project (or anyone else, for that matter!).

Basically, I'm not a fan of using the JSON enum for consuming the Twitter API. It's much better than having to cast from Any all over the place, but it's still pretty inelegant--especially compared to what you can get away with in weakly-typed languages like JavaScript.

So, in parallel with a personal project of mine, I've been writing a wrapper API for Swifter, built on top of the Swift 4 Decodable protocol (actually originally built in private on top of the JSON type, then switched to Decodable). I intend to continue working on it no matter what, but I'm willing to go the extra mile and add support for all possible interactions with the API if it's decided that that would be within scope for the project.

The main alternative I can think of to building this wrapper (other than doing nothing) is the following:

• Provide generic methods for all API endpoints, which take generic success handlers, which take in arguments of type T where T: Decodable
• Defer creation of wrapper types to the user

Lastly, as of right now, all timeline and tweet methods have been implemented, and both demo apps have been updated to use the wrapper instead of the JSON API. Please feel free to ask questions, and give any feedback you have time to.

Thanks!

[meteochu]

Hi, thanks again for the contribution. I've actually been thinking about this since Codable was first announced in a Swift Evolution some months ago. From a framework standpoint, I do not think that Swift should provide any struct/class that wraps around the Twitter classes. All Swifter is, is a Twitter API framework that gets the data in a neat fashion; rather, all wrapper classes should be separate from the core project.

What I believe should be done is that we just have a flag (either static or while calling each function) that asks the user if they would like to get a Data struct or the JSON object that we already wrap for them. Plus, if at all necessary, we could always just add a data property on the JSON object to get the raw data (in which they can use Codable to create their own objects).

[ghost]

Thanks for your input, Andy. I can completely understand wanting to limit the project to what you described above. I have an idea of an interesting solution that may or may not work; I’ll report back here soon.

[ghost]

OK, so now I know for sure my idea can work. Here goes, let me know what you think:

• Have JSON conform to Decodable (already done; jsonRequest now uses JSONDecoder to create the JSON objects that it passes to JSONSuccessHandlers)
• Add one or more (in the case of progress handlers) generic types to each function that returns data through a closure
• Add a parameter to those functions allowing users to specify the meta-type of the Decodable object they want in their success handler
• The default value can be JSON.self, allowing for full (or close to full) source compatibility
• Probably more stuff, I'm tired

[zntfdr]

I'm also a fan of Codable so much that I've completely removed the JSON enum in favor of the new Swift protocol in my other projects (I also wrote a post about it).

However, as @meteochu says, Swifter doesn't strive to provide structs/classes that wrap around the Twitter ones, this is something that should happen on your side of the project (outside the framework).

Making JSON conform to Decodable is a nice middleground though 😄

Edit
please note that these lines can be merged like this:

let jsonResult = try JSONDecoder().decode(JSON.self, from: data)

and

let result = try JSONDecoder().decode(T.self, from: data)

[ghost]

Thanks, Federico. Your post was one of the first I came across when I was looking into Codable, didn't realize it was you who wrote it until now, though! Great stuff.

And thanks for the correction. What I might end up doing is just having the Swifter class hold on to an instance of JSONDecoder and use that for all decoding.

[ghost]

The Swift compiler seems to dislike default values for metatype parameters, claims Default argument value of type 'JSON.Type' cannot be converted to type 'T.Type'.

For now, unless there's some other way to do this I haven't thought of, the meta-type parameters will have to be mandatory. If compatibility with old code is important enough, we can just add second versions of each of the public success-handler methods which call the generic versions with JSON.self. Annoying, yes, but not the end of the world. For now though, I'll just be focusing on the generic versions.

[ghost]

Good news! All endpoint methods now support the Decodable protocol, and any existing code that expects JSON objects should work completely unchanged. The demo apps, at the very least, work as expected.

One important note I should make is that all methods which extract data from JSON objects (such as the streaming API methods which decide which handler to call based on the format of the data) were scrapped in favour of using simple success handlers. The specialized JSON methods still extract this data and thus should continue to work as they did before. I originally attempted to recreate this behaviour for the Decodable API, but because Swift doesn't necessarily specialize generic types at compile time, it wouldn't be possible without changing the jsonRequest method to support one of multiple possible types for each handler, and that would get really messy.

Let me know your thoughts, guys.

[ghost]

Any comments or questions so far?

[meteochu]

On first glance, it seems a bit messy; I'm hoping for a cleaner implementation than our current one because otherwise there's no point in using Decodable if it's messier. I'll look over it when I have time (can't guarantee when).

[ghost]

Do you mean it imposes messiness on the user’s code, or that the implementation itself is messy (or both)? In either case I can explain why I did what I did, but in general I had to bend over backwards in order to ensure any pre-Decodable Swifter code continues to work unchanged. Perhaps that’s not necessary though?

[meteochu]

Both. We shouldn't use a new API just for the sake of using a new API; it needs to make the client's implementation easier, which in this case I'm doubting. Swifter is a framework, which means we need to ensure its flexibility for people using it. IMO using Decodable locks the client down and forces them to use the API even if their implementation doesn't use it, which means we'll be breaking a lot of projects by going with it. Furthermore, making JSON conform to Decodable like an hack and not what the API is supposed to be used for. It's meant for type-safety decoding of JSON of a known schema. Our current JSON implementation ensures that the the type of the JSON object can always be checked via an if case statement, and makes it really flexible.

My current preferred solution is just to continue passing the JSON object like we are, and also provide a new rawData property to extract the rawData if they want to use Decodable on the client side, or a generic way of specifying if they want a JSON or Data object in the success handler, which shouldn't be too hard with generic conformance.

[ghost]

Ok, sorry for the long message but there’s a lot to respond to here.

“It doesn’t make the client’s implementation easier”: It’s true that when calling Swifter methods you have to specify a type for the decoder to decode to. But that would be true of any possible implementation of Decodable for this library. Either you add a meta-type parameter or you force the user to specify it in their closure. Technically non-negotiable. I would’ve opted for the latter, except for doing so would break existing non-Decodable code because then the compiler would get confused as to which overload to use. I actually like that option a lot better as we could get rid of the method duplication I added, but again, it would break code so I didn’t do it. I also think you’re overlooking the ergonomic benefit of having type-safe wrapper types to work with.

“Using Decodable locks the client down to using it”: I don’t understand what you mean. What could a user possibly want to do with the JSON data other than decode it in some way? And what projects would be broken by going in this direction? All changes to the public interface are additive.

“Making JSON conform to Decodable is a hack and is not what it’s meant for”: I say it’s a hack that works. It allows all code to continue working as-is, while also giving users who want to customize their experience to do so with custom wrapper types. It’s not extensively tested so maybe there’s a bug or two in my implementation; but ignoring that I don’t see any way that this approach would necessarily produce different results, be significantly less performant, or be more difficult to maintain. JSONDecoder uses the same JSONSerialization type as Swifter does under the hood, after all.

“Our current JSON implementation ensures that the type of the JSON object can always be checked...”: Can you elaborate on how my implementation changes this? Any valid JSON value should be placed into the same enum case as it was before. All I did was add an initializer that takes a decoder.

“My current preferred solution is...”: Both proposed solutions have the problem that they require the user to manually decode the data and catch any potential decoding errors in their success and progress handlers (rather than in the error handlers), which I think is undeniably more inconvenient than merely specifying the type on method call. More on that below, though.

“...provide a new rawData property...”: There’s no efficient way (that I can think of) to handle this in Swift. Either you make rawData a computed property and waste CPU cycles, or you wrap JSON in a struct with a rawData stored property and waste memory.

“...a generic way of specifying if they want a JSON or Data object...”: This has the same problem my solution has. You’d still be forced to specify a type–this time either JSON or Data–but also would still have to decode and handle errors in the data.

[meteochu]

Our current implementation is type-safe too. You'll only ever get a value you want to fetch if the type is correct; it'll never hand you the wrong kind of value. Having rawData as a computed property isn't so bad either; it'll only ever get calculated once. Plus, from the completion handler, we can just attach the raw data (pre-JSON-prased) into the success handler: 0 wasted CPU cycles. Plus, leaving the client to decode the data themselves allows them the flexibility to handle how they want to catch the data.

My opinion is that Swifter is simply a framework for fetching Twitter data with convenience. We leave the flexibility and options of implementation to the user, we just need to get the data, and feed it to the user. That's all. They handle the rest. As to how we feed it to them, our current implementation is safe and convenient enough that there's no problems with type-safety, plus, it's extremely convenient to use.

Upon further inspection, what you're doing in JSON is the same as what we do before, except you're just using Decodable to do it. Sure, it's additive, but it's now adding duplicated functionality onto the existing framework, which is unnecessary.

[ghost]

It seems we'll have to agree to disagree, Andy. Thank you for sharing your thoughts

[meteochu]

I sincerely apologize if I've wasted your time, but thank you again for your contributions and efforts.

[ghost]

No apology necessary! I would've done this work anyway. I'm just unwilling to compromise on the core of how I've implemented this. That's the great thing about open source software. :)

[lukestringer90]

@zachrwolfe Is your fork still around? I would like the ability to decode JSON into my own model objects rather than use the provided JSON enum type. Seems like you solved this problem in the PR?

[ghost]

@lukestringer90 Unfortunately no, or at least not publicly. I tried to get the discussion going again in #204, but gave up after having trouble soliciting feedback from the maintainers of Swifter. I plan to open-source my fork eventually, but it's not ready at this time. If you'd like, you can email me at zachrwolfe at me dot com and I can share with you the version I'm working with?

[lukestringer90]

@zachrwolfe Great. I'll drop you an email.