> On 8 Jun 2017, at 17:48, Brent Royal-Gordon <br...@architechies.com> wrote:
>
>> On Jun 8, 2017, at 8:27 AM, Gwendal Roué via swift-evolution
>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:
>>>
>>> Le 8 juin 2017 à 16:51, James Froggatt via swift-evolution
>>> <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> a écrit :
>>>
>>> Here, container is stored as a `let` value, and uses reference semantics,
>>> while the proposal also clearly lists these `encode` methods as mutating.
>>> With the current implementation of the proposal, the container must be
>>> stored as a `var`, which leads to code like the following:
>>>
>>> var container = encoder.singleValueContainer()
>>> try container.encode(data)
>>
>> Yes, practically speaking and with latest Swift 4, the container needs to be
>> declared as `var`.
>>
>> I admit it's weird, and feels unnatural:
>>
>> public func encode(to encoder: Encoder) throws {
>> // A mutated value that nobody consumes: so weird.
>> var container = encoder.container(keyedBy: CodingKeys.self)
>> try container.encode(latitude, forKey: .latitude)
>> try container.encode(longitude, forKey: .longitude)
>> }
>>
>>> This clearly wont work as expected if the container were to have value
>>> semantics, and writing code like this feels plain wrong. Is SE-0166 really
>>> intended to work with referrence-type encoders only?
>>
>> Actually, it can work with encoder/containers that have value semantics, and
>> forward the mutations somewhere else (for example to a closure which fills a
>> mutating container).
>>
>> But this is again bizarre, and contrieved:
>> https://github.com/groue/GRDB.swift/blob/15bfe5f6cf76070cfb17216223bdebc6b158d654/GRDB/Record/Persistable%2BEncodable.swift
>>
>> <https://github.com/groue/GRDB.swift/blob/15bfe5f6cf76070cfb17216223bdebc6b158d654/GRDB/Record/Persistable%2BEncodable.swift>
>>
>> You make me think that those structs should swiftly be refactored into
>> reference types.
>
>
> I made precisely this comment during the review. The response was that the
> container could potentially defined by a struct that contains some state
> which it uses to write into a reference. For instance, imagine a type like
> this:
>
> class MyEncoder: Encoder {
> var buffer: Data
>
> struct KeyedEncodingContainer<K: CodingKey>:
> KeyedEncodingContainerProtocol {
> let encoder: MyEncoder
> var bufferOffset: Int
>
> mutating func encode(_ value: String, forKey key: K)
> throws {
> let newData = try makeDataFrom(value, forKey:
> key)
>
> encoder.buffer.insert(newData, at: offset)
> bufferOffset += newData.count
> }
> …
> }
> …
> }
>
> I argued that foreclosing designs like this would be acceptable—you could
> either make `KeyedEncodingContainer` a class or move `offset` into
> `MyEncoder` to get around it—but the proposers preferred to preserve this
> flexibility.
>
> --
> Brent Royal-Gordon
> Architechies
>
I see. In this context the documentation's list of preconditions makes sense,
though perhaps it could be updated to more directly explain this kind of use
case? This is an important detail to take into account, as it means that making
a copy of the container could lead to unexpected behaviour:
var container2 = container
container.encode(data1, forKey: CodingKeys.key1)
container2.encode(data2, forKey: CodingKeys.key2) // oops, what does .key1
point to now?
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution
