<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">(If you'd like any further discussion to move over to the review thread, please let me know.)</div><br class=""><div><blockquote type="cite" class=""><div class="">On Apr 5, 2017, at 9:08 PM, Itai Ferber <<a href="mailto:iferber@apple.com" class="">iferber@apple.com</a>> wrote:</div></blockquote><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div class=""><div style="font-family:sans-serif" class=""><div style="white-space:normal" class=""><div class=""></div><p dir="auto" class="">In terms of an equivalent to <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7" class="">encode(contentsOf:)</code>, keep in mind that this would only work if the collection you're decoding is homogeneous, in which case, you would likely prefer to decode an <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7" class="">Array</code> over getting an unkeyed container, no? (As soon as conditional conformance arrives in Swift, we will be able to express <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7" class="">extension Array : Decodable where Element : Decodable { ... }</code> making decoding homogeneous arrays trivial.)</p><div class=""></div></div><div style="white-space:normal" class=""><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px" class=""></blockquote></div></div></div></div></blockquote>That's true (and I assumed that `Array`s and friends would be `Codable`—we don't even need to wait for conditional conformances ), but it's hardly unheard of to write your own `Collection` types when you need different semantics.</div><div class=""><br class=""></div><div class="">Swift has two mutation protocols that are important for this purpose: `RangeReplaceableCollection` and `SetAlgebra`. You could provide methods on `UnkeyedDecodingContainer` that work with them:</div><div class=""><br class=""></div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>func decodeAll<C: RangeReplaceableCollection>(_ type: C.Type) throws -> C where C.Iterator.Element: Encodable {</div><div class=""><span class="Apple-tab-span" style="white-space:pre">                </span>var collection = C()</div><div class=""><span class="Apple-tab-span" style="white-space:pre">                </span>if let capacity = self.count {</div><div class=""><span class="Apple-tab-span" style="white-space:pre">                        </span>collection.reserveCapacity(capacity)</div><div class=""><span class="Apple-tab-span" style="white-space:pre">                </span>}</div><div class=""><span class="Apple-tab-span" style="white-space:pre">                </span>while !self.isAtEnd {</div><div class=""><span class="Apple-tab-span" style="white-space:pre">                        </span>collection.append(try self.decode(C.Iterator.Element.self))</div><div class=""><span class="Apple-tab-span" style="white-space:pre">                </span>}</div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>}</div><div class=""><br class=""></div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>func decodeAll<S: SetAlgebra>(_ type: S.Type) throws -> S where S.Element: Encodable {</div><div class=""><span class="Apple-tab-span" style="white-space:pre">                </span>var set = S()</div><div class=""><span class="Apple-tab-span" style="white-space:pre">                </span>while !self.isAtEnd {<div class=""><span class="Apple-tab-span" style="white-space: pre;">                        </span>set.insert(try self.decode(C.Iterator.Element.self))</div><div class=""><span class="Apple-tab-span" style="white-space: pre;">                </span>}</div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>}</div><div class=""><br class=""></div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>// Usage:</div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>let array = container.decodeAll(ContiguousArray<Pet>.self)</div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>let set = container.decodeAll(Set<Pet>.self)</div></div></div></div></blockquote></div></div></div></blockquote><blockquote type="cite" class=""><br class=""></blockquote><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">Potentially, thought it would be ambiguous if you had a collection that conformed to both.<br class=""></div></div></div></blockquote><div><br class=""></div><div>I'm fairly certain their semantics are mutually exclusive. RangeReplaceableCollection is for collections that allow you to place arbitrary elements at arbitrary indices; SetAlgebra is for collections (and some non-collection types, like OptionSets) which deduplicate elements when they're inserted. I don't think a type could validly conform to both.</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>// Usage:</div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>let array = container.decodeAll(ContiguousArray<Pet>.init)</div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>let set = container.decodeAll(Set<Pet>.init)</div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span></div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>// I'm imagining here a future version of Swift where it's possible to </div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>// make tuples of `Codable` types `Codable` themselves.</div><div class=""><span class="Apple-tab-span" style="white-space:pre">        </span>let dictionary = container.decodeAll(Dictionary<Pet, Person>.init)</div></div></div></div></blockquote></div></div></div></blockquote><blockquote type="cite" class=""><br class=""></blockquote><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">These are interesting approaches, and I’ll explore them a bit. Just keep in mind that the bar for adding them as public API that we’ll have to maintain is pretty high; with such a large API surface already, these would have to prove significantly beneficial in practice.<br class=""></div></div></div></blockquote><div><br class=""></div>I understand. Thanks for looking at this.</div><div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Yeah, that makes sense. To make that work, though, you might end up having a top-level `Decodable` type read a `version` field or something and write it into the `userInfo` for other types to examine. Nothing necessarily wrong with that, I suppose.</div><div class=""><br class=""></div><div class="">(Hmm...coming back after looking at your `JSONDecoder`, it looks like it might not be possible to do this, or perhaps that it could only be done by storing a reference type into the `userInfo` before you started writing and modifying it during decoding. Is that intentional?)<br class=""></div></div></div></blockquote></div></div></div></blockquote><blockquote type="cite" class=""><br class=""></blockquote><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><font face="Menlo" class="">userInfo</font> dictionaries in general should not be modified during encoding — that feels like a dependency injection attack waiting to happen. (What do you do when another type you don’t control can inspect the <span style="font-family: Menlo;" class="">userInfo</span> dictionary and starts messing with your values, either intentionally or not?)</div><div class="">If you really must I guess passing a shared reference type in there is possible, but that’s something we’d like to discourage.</div><div class=""><br class=""></div><div class="">If your type needs this information it should either write it as part of its representation (your type can have its own version field if need be) [most common case], or grab this information if known at the top level before going to decode [applicable in certain instances]. If your type depends on the dynamic state of other types during decoding, you’ll want to sideband that information safely, out of reach of other code.<br class=""></div></div></div></blockquote><div><br class=""></div><div>If you're worried about that, I'd suggest a different design for `userInfo`:</div><div><br class=""></div><div>* Instead of using struct-wrapped string keys, use opaque object keys. You can keep a key `private` if you want it to be a private side channel.</div><div><br class=""></div><div>* Don't use (or at least expose) an actual `Dictionary` for `userInfo`; instead, either expose a type which can be keyed but not iterated over, or just expose a `userInfo(forKey:)` method.</div><div><br class=""></div><div>Then you can control access to a given piece of `userInfo` by controlling access to the key that accesses it.</div><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="font-family:sans-serif" class=""><div style="white-space:normal" class=""><p dir="auto" class="">
If you really must try to decode regardless, you can always try to grab one container type from the decoder, and if it fails, attempt to grab the other container type.</p><div class=""></div></div><div style="white-space:normal" class=""><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px" class=""></blockquote></div></div></div></blockquote>But I assume that some decoders might just misinterpret whatever is present? For instance, I could imagine an encoder which doesn't natively support keyed containers, so it fakes it by alternating key and value fields in a flat list; if you wrote with a keyed container and read with an unkeyed container, or vice versa, it would then happily misinterpret whatever was present. So it seems like this would probably be a pattern we'd want to discourage.<br class=""></div></div></div></blockquote></div></div></div></blockquote><blockquote type="cite" class=""><br class=""></blockquote><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">If an <font face="Menlo" class="">Encoder</font> does not natively support any one of the features on this API (likely because of the format), it should write data in a way that it can unambiguously understand on the decode side. Whether this means writing a magic identifier or something to the head of the flat list to know to interpret it as a dictionary, or similar, it needs to be able to understand its own data. If this is not possible, then it seems that the <font face="Menlo" class="">Encoder</font>/format is a poor fit for this API.<br class=""></div></div></div></blockquote><div><br class=""></div>Noted.<br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="font-family:sans-serif" class=""><div style="white-space:normal" class=""><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px" class=""><p dir="auto" class="">* I think it may make sense to class-constrain some of these protocols. `Encodable` and its containers seem to inherently have reference semantics—otherwise data could never be communicated from all those `encode` calls out to the ultimate caller of the API. Class-constraining would clearly communicate this to both the implementer and the compiler. `Decoder` and its containers don't *inherently* have reference semantics, but I'm not sure it's a good idea to potentially copy around a lot of state in a value type.</p></blockquote></div>
<div style="white-space:normal" class=""><p dir="auto" class="">I don't think class constraints are necessary. You can take a look at the current implementation of <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7" class="">JSONEncoder</code> and <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7" class="">JSONDecoder</code> <a href="https://github.com/itaiferber/swift/blob/3c59bfa749adad2575975e47130b28b731f763e0/stdlib/public/SDK/Foundation/JSONEncoder.swift" style="color:#3983C4" class="">here</a> (note that this is still a rough implementation and will be updated soon). The model I've followed there is that the encoder itself (<code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7" class="">_JSONEncoder</code>) has reference semantics, but the containers (<code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7" class="">_JSONKeyedEncodingContainer</code>, <code style="background-color:#F7F7F7; border-radius:3px; margin:0; padding:0 0.4em" bgcolor="#F7F7F7" class="">_JSONUnkeyedEncodingContainer</code>) are value-type views into the encoder itself.</p><p dir="auto" class="">Keep in mind that during the encoding process, the entities created most often will be containers. Without some additional optimizations in place, you end up with a <em class="">lot</em> of small, short-lived class allocations as containers are brought into and out of scope.<br class="">
By not requiring the class constraints, it's at least possible to make all those containers value types with references to the shared encoder.</p><div class=""></div></div><div style="white-space:normal" class=""><blockquote style="border-left:2px solid #777; color:#777; margin:0 0 5px; padding-left:5px" class=""></blockquote></div></div></div></blockquote>Okay, I see what you're getting at. But that still makes me think that:</div><div class=""><br class=""></div><div class="">* `Encoder` and `Decoder` should be class-constrained. An `Encoder` *must* share some kind of reference with its containers, and while it would certainly be possible to hide that reference away somewhere inside a value-typed `Encoder`, I can't imagine it's worth penalties like having to pass a larger existential container (`class`-constrained protocol existentials are 2+ words instead of 5+) and having to virtualize reference-counting operations through a value witness table. (See <a href="https://github.com/apple/swift/blob/master/docs/ABI.rst#class-existential-containers" class="">https://github.com/apple/swift/blob/master/docs/ABI.rst#class-existential-containers</a> for details here.)</div></div></div></blockquote></div></div></div></blockquote><blockquote type="cite" class=""><br class=""></blockquote><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class="">The <font face="Menlo" class="">Encoder</font> and <font face="Menlo" class="">Decoder</font> need not be class constrained, for the same reasons. Somewhere down the line, something must have reference semantics, you’re right, but the thing which is shared need not be the <font face="Menlo" class="">Encoder</font> either.</div><div class=""><br class=""></div><div class="">For instance, consider one possible design of a streaming encoder. The <font face="Menlo" class="">Encoder</font> and its containers may all be structs with an offset into a data stream or blob; writing to a container simply appends to the stream. It’s not necessarily the <font face="Menlo" class="">Encoder</font> that needs to be shared, but potentially the underlying data store. We wouldn’t want to enforce a design choice that would prevent this kind of setup if it benefitted the encoder and its format.</div></div></div></div></blockquote><div><br class=""></div><div>I think we're on the same page on these points:</div><div><br class=""></div><div>* All valid encoder designs will have reference semantics.</div><div>* Most encoder designs must be classes (or at least classes are *by far* the easiest design).</div><div>* Some encoder designs could be structs or enums with an embedded reference of some kind.</div><div><br class=""></div><div>Even though some encoders could be designed as a struct, I don't think it's worth it to support that. Besides value semantics, there are two reasons to prefer a struct:</div><div><br class=""></div><div>* All member invocations on structs are static.</div><div>* Structs don't need to be reference counted.</div><div>* You can directly access them.</div><div><br class=""></div><div>But in practice, those benefits are not available here. `Encodable` always passes the `Encoder` in a type-erased existential container, so all invocations will be done virtually through the protocol witness table. Implementations have to assume that the `Encoder` might be an object, so they will have to make a *virtual* call through the value witness table to copy/retain the instance, and another to release it. And non-class-constrained existential containers have to be passed indirectly, since their size and contents aren't known to the compiler.</div><div><br class=""></div><div>The upshot of all this is, I really doubt there will be any practical benefit to using a struct, and doing so may leave performance gains on the table.</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">* There is no need for the `encode` calls to be marked `mutating`. An encoding container is just a view on state within a reference type, so it doesn't mutate the container itself, only the portion of the encoder the container refers to. (It is not possible for an encoding container to hold value-typed state, either, because the encoder never gets a chance to grab that state once you're finished with it.) While these calls do mutate *some* state *somewhere*, they don't mutate the state of the container.</div></div></div></blockquote></div></div></div></blockquote><blockquote type="cite" class=""><br class=""></blockquote><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class="">Again, not necessarily true. The container may want to maintain state on its own that is not shared with the <font face="Menlo" class="">Encoder</font>. You can imagine a container which queues up all of the encode calls it receives and builds up a mini payload, and only produces data once the <font face="Menlo" class="">Encoder</font> finalizes the container, or similar.</div></div></div></div></blockquote><div><br class=""></div><div>No, actually, I can't imagine such a container. If the container is a value type, then once it is returned from `container(keyedBy:)` or its siblings, the `Encoder` has no idea where it is located or even how many copies of it might have been made. There's no way for the encoder to get at the mini-payload unless it's stored at some stable memory address, in which case the `encode` calls don't need to be mutating to write to it. You could imagine some sort of state being stored in the container—a "next value offset" field, for instance—but then writing from different copies of the container (for instance, in a helper method which didn't get the container through an `inout` parameter) would end up overwriting existing data and probably losing or corrupting the encoded data. Maybe if the container were `moveonly`, that could be done—but `moveonly` is a hypothetical future feature and making the containers `moveonly` in the future would break existing code.</div><div><br class=""></div><div>I'm sorry if I'm not explaining this well, but I think that if you try to actually build a toy encoder with value-typed containers, you'll see what I mean. You might even see it just by thinking through the implementation in detail.</div><div><br class=""></div><div>Even if a container is implemented with a value type, it *must* have reference semantics or it will not behave as users expect. We can encourage that by making these methods non-mutating, which means that any state they mutate has to be accessed by reference.</div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Basically, what I'm saying is that `Encoder` should always be a reference *type* and encoding containers, even if implemented with a value *type*, should always have reference *semantics*.</div><div class=""><br class=""></div><div class="">(The `decode` calls on an `UnkeyedDecodingContainer` are a harder case, because it really *is* plausible that the "next element" pointer might be stored in the container rather than the decoder. I'm not totally sure what to do there.)</div></div></div></blockquote></div></div></div></blockquote><blockquote type="cite" class=""><br class=""></blockquote><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">Yes, this is the type of local state that’s needed. <font face="Menlo" class="">UnkeyedDecodingContainer</font>, for instance, may have an index into the array that it holds. Every decode call will need to increment that index by one. Less likely on encode, but still possible.</div></div></div></blockquote><div><br class=""></div><div>True, but that does mean that, if you copy a decoder, you may or may not be able to enumerate its contents more than once. If you make `decode(_:)` nonmutating, then it's only possible to implement one semantic: you can only read each element once.</div><div><br class=""></div><div>The standard library's `IteratorProtocol` faces a similar issue: because it can implement either value or reference semantics, you can't reliably know what will happen if you copy an iterator. My sense is that there's growing agreement that this state of affairs is not good, but `IteratorProtocol`'s speed requirements are so extreme that we can't afford any of the solutions the language currently offers. `UnkeyedDecodingContainer` is not in the same position—if it were, we wouldn't use it in type-erased form.</div><div><br class=""></div><div>But again, even if this is true for the *de*coding container, that doesn't mean it is for the *en*coding container. Encoding is intrinsically backed by a type with reference semantics. The same is not true for decoding.</div><br class=""><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">The point I want to get at though is that "keyed vs. unkeyed" can be somewhat more of a mindset, and once you learn that keyed is almost always what you want, that’s something you reach for instinctively, and consistently. Learning to use the keyed container is something you learn once, and that’s what you’ll keep reaching for.</div></div></blockquote><div><br class=""></div><div>To be clear, I don't criticize the decision to put only containers at the top level; I just wanted to call it out as part of the user's experience as they're feeling their way through this type.</div><br class=""><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">What you can <i class="">do</i> with a keyed container, though, is something you will want to review on a case-by-case basis. Having the list of overloads in place helps reinforce that some of these types are special over others, and being able to review that list concretely is nice, even if just for reference.</div></div></blockquote><div><br class=""></div><div>But why does the user care that these particular types are special? Why is it important that `String` is primitive but `URL` is not? Are the primitives really "fifteen of sixteen overloads in a list we're going to present constantly" important?</div><div><br class=""></div><div>To look at it another way: Think about the capabilities that we *aren't* showing because the list is so long. Is our support for UInt16 more important than our support for nested containers or super encoders?</div><br class=""><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><div class=""><div class=""><div class=""><div style="font-family: sans-serif; white-space: normal;" class="">If we wanted the overload list to help beginners understand which types `encode(_:forKey:)` supports, I would want it to look more like:</div><div style="font-family: sans-serif; white-space: normal;" class=""><br class=""></div><div class=""><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>[CodingKey?]<span class="Apple-tab-span" style="white-space: pre;">                                                </span>codingPath</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encode(value: [Encodable]?, forKey: MyModel.CodingKeys) throws</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encode(value: [Encodable & Hashable: Encodable]?, forKey: MyModel.CodingKeys) throws</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encode(value: Data?, forKey: MyModel.CodingKeys) throws</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encode(value: Date?, forKey: MyModel.CodingKeys) throws</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encode(value: Double?, forKey: MyModel.CodingKeys) throws</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encode(value: Encodable?, forKey: MyModel.CodingKeys) throws</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encode(value: Int?, forKey: MyModel.CodingKeys) throws</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encode(value: Set<Encodable & Hashable>?, forKey: MyModel.CodingKeys) throws</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encode(value: String?, forKey: MyModel.CodingKeys) throws</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encode(value: URL?, forKey: MyModel.CodingKeys) throws</div><div style="font-family: sans-serif; white-space: normal;" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Void<span class="Apple-tab-span" style="white-space: pre;">                                                                </span>encodeWeak(object: (AnyObject &…), forKey: MyModel.CodingKeys) throws</div><div style="font-family: Helvetica; white-space: normal;" class=""><span class="Apple-tab-span" style="font-family: sans-serif; white-space: pre;">        </span><font face="sans-serif" class="">KeyedEncodingContainer<CodingKey><span class="Apple-tab-span" style="white-space: pre;">        </span>nestedContainer(keyedBy: CodingKey.Protocol, forKey: MyModel.CodingKeys)</font></div><div style="font-family: Helvetica; white-space: normal;" class=""><font face="sans-serif" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>UnkeyedEncodingContainer<span class="Apple-tab-span" style="white-space: pre;">                        </span>nestedUnkeyedContainer(forKey: MyModel.CodingKeys)</font></div><div style="font-family: Helvetica; white-space: normal;" class=""><font face="sans-serif" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Encoder<span class="Apple-tab-span" style="white-space: pre;">                                                        </span>superEncoder()</font></div><div style="font-family: Helvetica; white-space: normal;" class=""><font face="sans-serif" class=""><span class="Apple-tab-span" style="white-space: pre;">        </span>Encoder<span class="Apple-tab-span" style="white-space: pre;">                                                        </span>superEncoder(forKey: </font><span style="font-family: sans-serif;" class="">MyModel.CodingKeys)</span></div><div style="font-family: sans-serif; white-space: normal;" class=""><span style="font-family: sans-serif;" class=""><br class=""></span></div><div style="font-family: sans-serif; white-space: normal;" class=""><span style="font-family: sans-serif;" class="">These ten overloads more fairly represent the kinds of Swift- and Foundation-provided encodable types people actually use most often. The presence of `Array`, `Dictionary`, and `Set` convey that encoded types may have a complex inner structure. The absence of all the `Int`/`UInt` varieties does mean it's not immediately obvious that it supports every single one, but Swift guidelines highly discourage use of those types anyway unless you *really* need those semantics for some reason.</span></div></div></div></div></div></div></div></div></blockquote></div></div></blockquote><blockquote type="cite" class=""><br class=""></blockquote><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class="">Sure, I don’t disagree that this list would likely be most helpful for beginners. But on the other hand, it doesn’t correctly reflect the actual interface on an encoding container, which is more important in general.</div></div></div></blockquote><div><br class=""></div><div>Sure, but we choose what the "actual interface" is. It can be mostly primitive types, it can be explanatory, or it can be one flexible generic method.</div><div><br class=""></div><div>I think we've drifted a little bit from the topic. To quote you from a few posts up the thread:</div><div><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space;"><div class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><div class=""><div class="" style="font-family: sans-serif;"><div class=""><p dir="auto" class=""></p></div></div></div></div></div></div></div></div></div><blockquote type="cite" class=""><div><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space;"><div class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><div class=""><div class="" style="font-family: sans-serif;"><div class=""><p dir="auto" class=""></p></div></div></div></div></div></div></div></div></div></blockquote><blockquote type="cite" class=""><blockquote type="cite" class=""><div><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space;"><div class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><div class=""><div class="" style="font-family: sans-serif;"><div class=""><p dir="auto" class=""></p></div></div></div></div></div></div></div></div></div></blockquote></blockquote><blockquote type="cite" class=""><blockquote type="cite" class=""><blockquote type="cite" class=""><div><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space;"><div class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><div class=""><div class="" style="font-family: sans-serif;"><div class=""><p dir="auto" class="">Think of the experience for the consumer of this API, especially someone learning it for the first time. It can already be somewhat of a hurdle to figure out what kind of container you need, but even once you get a keyed container (which is what we want to encourage), then what? You start typing <code bgcolor="#F7F7F7" class="" style="background-color: rgb(247, 247, 247); border-top-left-radius: 3px; border-top-right-radius: 3px; border-bottom-right-radius: 3px; border-bottom-left-radius: 3px; margin: 0px; padding: 0px 0.4em;">container.enc...</code> and in the autocomplete list in Xcode, the only thing that shows up is one autocomplete result: <code bgcolor="#F7F7F7" class="" style="background-color: rgb(247, 247, 247); border-top-left-radius: 3px; border-top-right-radius: 3px; border-bottom-right-radius: 3px; border-bottom-left-radius: 3px; margin: 0px; padding: 0px 0.4em;">encode(value: Encodable, forKey: ...)</code> Consider the clarity (or lack thereof) of that, as opposed to seeing <code bgcolor="#F7F7F7" class="" style="background-color: rgb(247, 247, 247); border-top-left-radius: 3px; border-top-right-radius: 3px; border-bottom-right-radius: 3px; border-bottom-left-radius: 3px; margin: 0px; padding: 0px 0.4em;">encode(value: Int, forKey: ...)</code>, <code bgcolor="#F7F7F7" class="" style="background-color: rgb(247, 247, 247); border-top-left-radius: 3px; border-top-right-radius: 3px; border-bottom-right-radius: 3px; border-bottom-left-radius: 3px; margin: 0px; padding: 0px 0.4em;">encode(value: String, forKey: ...)</code>, etc. Given a list of types that users are already familiar with helps immensely with pushing them in the right direction and reducing cognitive load. When you see <code bgcolor="#F7F7F7" class="" style="background-color: rgb(247, 247, 247); border-top-left-radius: 3px; border-top-right-radius: 3px; border-bottom-right-radius: 3px; border-bottom-left-radius: 3px; margin: 0px; padding: 0px 0.4em;">String</code> in that list, you don't have to question whether it's possible to encode a string or not, you just pick it. I have an <code bgcolor="#F7F7F7" class="" style="background-color: rgb(247, 247, 247); border-top-left-radius: 3px; border-top-right-radius: 3px; border-bottom-right-radius: 3px; border-bottom-left-radius: 3px; margin: 0px; padding: 0px 0.4em;">Int8</code>, can I encode it? Ah, it's in the list, so I can.</p></div></div></div></div></div></div></div></div><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space;"><div class=""><blockquote type="cite" class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><blockquote type="cite" class=""><div class=""><div class="" style="font-family: sans-serif;"><div class=""><div class=""><blockquote type="cite" class=""></blockquote></div></div></div></div></blockquote><div class=""><div class=""><div class=""><div class="" style="font-family: sans-serif;"></div></div></div></div></div></div></div></blockquote></div></div></div></blockquote></blockquote></blockquote><div>My point is simply this: The particular list of types we'll end up showing is not well-suited to the purpose you describe here. You mention a beginner being reassured that `Int8` is in the list, but `Int8` is not a familiar, common type in Swift; it is a very rare type which idiomatic code will rarely use. Of the types that will be shown, only `Bool`, `Data`, `Double`, `Int`, and `String` will be familiar and common to Swift users.</div><div><br class=""></div><div>There is a separate reason to offer explicit overloads for these types—to encourage conforming types to implement fast handling of them—and, although I don't think that should be such a high priority, I do agree that your design is a good way to address that problem. But I don't think this "teach people how to use the API through autocomplete" justification makes sense, because the list of types just isn't a good fit for that task.</div><br class=""><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">Unless you mean that these types are not handled specially in the first place, and you let them go through the regular generic path so that they request a single value container on their own… This is possible, but you’d be missing out on an enormous optimization. Every single primitive encode would go through <font face="Menlo" class="">container.encode(…, forKey:…)</font> → create a new <font face="Menlo" class="">Encoder</font> reference → <font face="Menlo" class="">PrimitiveType.encode(to:)</font> → request a single value container from the <font face="Menlo" class="">Encoder</font> → <font face="Menlo" class="">container.encode(…)</font>, all instead of <font face="Menlo" class="">container.encode(…, forKey:…)</font> → store primitive value directly.</div><div class=""><br class=""></div><div class="">Even in the most optimized of cases (where the container already references an encoder, and the encoder conforms to <font face="Menlo" class="">SingleValueContainer</font> directly such that it can just <font face="Menlo" class="">return self</font>), you’re paying for 3 extra method calls (at least one dispatching through an existential) on every single primitive encode.</div><div class=""></div></div></blockquote><div><br class=""></div><div>Yeah, I was about to point out that your own JSON encoder doesn't create a new encoder *or* a new container; it just pushes a key onto an array (which will almost always have enough capacity to avoid an allocation) and passes itself to `encode(to:)` on the value, which requests a container and gets `self` back. </div><br class=""><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">So in the worst case, if you don’t switch on the type, it will just be slow. If the default implementation is not appropriate for a given type, though, nothing will be in place to catch it if you forget to implement that...<br class=""></div></div></blockquote><div><br class=""></div>Unless I'm mistaken, the behavior of `encode(_: Int, forKey: Key)` *must* be compatible with the behavior of `encode<T: Encodable>(_: T, forKey: Key)` when `T` is an `Int`, since you never know when somebody is going to write code that happens to pass through the generic overload instead of the primitive overload. Since I'm not suggesting we remove the primitives from the single value container, that means the worst case *is* that, if you forget to explicitly handle a primitive type, it will default to a code path that's somewhere between three method calls slower, and three method calls plus two allocations slower.</div><div><br class=""><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><blockquote type="cite" class=""><div class=""><div class="" style="font-family: sans-serif;"><div class=""></div></div></div></blockquote></div></div></div></blockquote></div><div class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Apropos of none of this discussion so far, one other thing I just noticed: `JSONEncoder` and `JSONDecoder` have a lot of parallel "strategy" properties, but they use different types for them. Have you considered making a single `JSONCodingStrategy` type which expresses bidirectional rules? That would allow a developer to make a constant containing their strategy and use it as a single source of truth.</div></div></div></blockquote></div></div></blockquote><blockquote type="cite" class=""><br class=""></blockquote><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">This has been brought up in past review. It might seem alluring to combine these, but in practice, two out of the four strategies cannot be reasonably combined because their types differ (<font face="Menlo" class="">Dat{e,a}{En,De}codingStrategy</font> — their custom closures have differing types). It’s possible to create <font face="Menlo" class="">.custom</font> cases which take backward and forward closures, but then what if you only care about one direction? If the closures are optional, then it’s possible to give a <font face="Menlo" class="">.custom</font> case with two nil closures, or even just a nil closure for the wrong direction.</div><div class=""></div></div></blockquote><div><br class=""></div><div>In that case, I would either just make them specify two closures, or have a throwing (or trapping) default, but...</div><br class=""><blockquote type="cite" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">Overall, just a bit too much complexity to be worth it.</div></div></blockquote><br class=""></div><div>I understand this perspective, too.</div><br class=""><div class="">
<span class="Apple-style-span" style="border-collapse: separate; font-variant-ligatures: normal; font-variant-east-asian: normal; font-variant-position: normal; line-height: normal; border-spacing: 0px;"><div class=""><div style="font-size: 12px; " class="">-- </div><div style="font-size: 12px; " class="">Brent Royal-Gordon</div><div style="font-size: 12px; " class="">Architechies</div></div></span>
</div>
<br class=""></body></html>