[swift-evolution] [Review] SE-0167: Swift Encoders

[Jon Shier]

> What is your evaluation of the proposal?
-1. There is a lot good here, but this feature is too important to let an “okay” proposal through.

* Usage of the API doesn’t feel very Swifty, unless I’m missing some intended use case. Why are the encoders / decoders classes? Why do they have mutable properties? What is the userInfo value for? It all feels very reminiscent of the NSFormatter APIs, which isn’t really a good thing. This is supposed to be the Swift native way to encode and decode types, so it should feel native. It’s close in a lot of ways (I like the strategies enums) but isn’t really there. 

* This is more for 166, but decode(MyValue.self)? Gross. Use the generic to convey the type inside the API.

* This proposal fails to make full usage of the Swift’s error capabilities and the errors live in the wrong place. I’m putting this here since this proposal defines the errors but really they should be part of 166. These error types are inferior to the errors we can produce in Swift right now. They should be Swift native errors first and bridged into NSError second, not created with the primary intent to be bridged. For example:

public static var coderTypeMismatch: CocoaError.Code

First off, the coder* prefix is unnecessary if this is properly moved into its own concrete error type. Second, it doesn’t capture the mismatch values, leading to unnecessary debugging overhead. Make the error a proper enum that can capture the type found and type expected and it becomes far more useful. For example (roughly):

public enum DecoderError: Error {
    typeMismatch(expected:actual:)
    readCorrupt(underlyingError: Error) // presumably this wraps the underlying serialization error?
    valueNotFound(forKey: Key)
}

Also, these errors should live in the standard library, otherwise how are native types going do their own encoding/decoding without importing Foundation. Again, this is more for 166, but they were defined here, so… I understand the need to perhaps bridge them to existing errors (I don’t think it’s necessary, but I understand the desire), but that should be done under the covers, so to speak, and not exposed to Swift developers by limiting a core API so badly. Besides, the errors you want to bridge to aren’t very good in the first place. "The data couldn't be read because it isn't in the correct format.” Seriously? Could we get a little more detail, please?

* There is nothing proposed to add any type safety to the decoding process. We know exactly what types can exist in JSON. Why are we still representing it as Any or the raw Data? Providing greater safety during the decoding process helps developers find what’s wrong faster.

> Is the problem being addressed significant enough to warrant a change to Swift?
Yes, we need native decoding and encoding of common formats like JSON, but I don’t believe this solution is good enough.

> Does this proposal fit well with the feel and direction of Swift?
No, this feels like a port of an Objective-C API, and it literally is in some ways.

> If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
I’ve been using Argo for 2 years now. While much of the comparison is more relevant to 166, I think it’s still a great example of what a Swift native decoding experience should look like. It’s especially superior in the errors it produces.

> How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Fairly in depth I’d say.



Jon Shier

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-evolution/attachments/20170412/980a82fb/attachment.html>