search

a2 / MessagePack.swift

It's like JSON, but fast and small…and Swift! – msgpack.org[Swift]
http://msgpack.org
MIT License
283 stars 60 forks source link

Support Codable without implementing the full Encoder and Decoder #63

Closed cherrywoods closed 6 years ago

cherrywoods commented 6 years ago

Hey, I don't know what's up on the other pull request about this topic, but I would like to submit another approach to this.

This implementation also supports Codable without copying all the code of JSONEncoder (directly). However it adds a dependency to the project: it depends on MetaSerialization (https://github.com/cherrywoods/swift-meta-serialization) that implements the Encoder and Decoder and KeyedEncodingContainer, and so on.

The code for adding this support in this repository is pretty short (but it's a bit ugly, because it's 70% type checking code), for the work it does.

The other thing I'd liked to suggest is providing such an interface to encode and decode Codable types to serialise to MessagePackValue, instead then to Data. This would allow to mix both ways of creating msg pack with the framework, by e.g. encoding some custom type and then using the returned MessagePackValue in another (array, e.g.) MessagePackValue. The fork also contains code to do this the other way around, for example having a MessagePackValue property and encoding this one, so it is embedded into the other MessagePackValues that are created on converting this custom type.

Anyway, I don't know if it would be better to create another repository for this project, or to merge it into yours.

cherrywoods commented 6 years ago

Well however, now also three tests for the new CodableConverter are included. The test run fine on my computer...

One of the included tests checks, whether the new class performs implicit loosely conversions from Double to Float. (It does) This behaviour can easily be changed to the opposite, because the implementation uses the floatValue computed property of MessagePackValue. If this implementation is changed from Float(_) to Float(exactly:), but changing this makes another test fail. The implementation in MessagePackValueTranslator can as easily be changed, I suppose.

Waiting for comments on this...

a2 commented 6 years ago

Hey @cherrywoods. I love the idea of using a "universal" Codable solution but I'm hesitant to bring a dependency into MessagePack.swift. Good catch on the implicit lossy conversion between floating point values. Merged #62 to address this.

cherrywoods commented 6 years ago

Thanks. I do understand your hesitation. I do also think, that it would be advantageous to provide this functionality as a separate framework.

a2 commented 6 years ago

@cherrywoods Agreed

cherrywoods commented 6 years ago

Ok. I'll wait for andrew-eng's suggestion and create a new repository then if necessary. (but I already have a repository that partially supports this, if someone is out there is watching us and needs this functionality right now to get a spaceship flying 🚀😄)

a2 commented 6 years ago

Going to close this PR since I think we agree that, although this PR achieves the goals of adding Codable support to MessagePack.swift, it isn't exactly what the project needs right now.