endocrimes / Jay

Pure-Swift JSON parser & formatter. Fully streamable input and output. Linux & OS X ready. Replacement for NSJSONSerialization.
MIT License
132 stars 19 forks source link

Offer a String API #55

Open remko opened 8 years ago

remko commented 8 years ago

Am I correct that the only JSON serialization format supported right now is raw data? I'm guessing this is because this is what JSONSerialization offers?

In practice, I find it annoying to have to work with raw data, as I always work with Strings when using JSON, and let later stages care about which encoding to put it on the wire. As far as I understand, this means I have to always run this through String:validatingUTF8 (and know that the raw data Jay returns is UTF-8, which is undocumented at the moment), and force-unpack the optional (which I can be 100% sure is valid utf-8 anyway); vice versa, it means I have to call .utf8 every time I want to parse something with Jay (which is slightly less annoying than the former).

Wouldn't it be good to offer an API that parses and serializes to Strings?

Also, the fact that the API only takes raw bytes means you have to document which encoding it is in and which encoding it outputs. JSONSerialization takes any encoding, Jay only seems to take UTF-8, (see #54 ).

czechboy0 commented 8 years ago

Actually, Jay should have public extensions to convert between [UInt8] and String, so it is just one method call to convert between them.

The new api would just do this conversion for you, nothing else. With the knowledge of the convenience conversion methods I mentioned above, would you still find an extra API so useful? It'd have to come in at least 4 variants, so it'd require additional tests and maintenance. But I don't feel strongly about it either way.

remko commented 8 years ago

I should probably have not filed 2 requests, because I'm probably giving everyone more work with my duplicate posts :)

As I said in #54 , I would personally even drop the [UInt8] API if you only take UTF-8 strings anyway, because it leaves room for errors on the user side. Only having a String API will make sure that, if all the user has is raw data, they at least know they have to think about which encoding it is in when the compiler tells them Jay needs Strings.

czechboy0 commented 8 years ago

There's a huge performance issue if we'd force people to convert to string when receiving data from the network when they want JSON. So I don't think we'll remove it.

remko commented 8 years ago

Actually, Jay should have public extensions to convert between [UInt8] and String, so it is just one method call to convert between them.

Such an API would mean you either:

remko commented 8 years ago

There's a huge performance issue if we'd force people to convert to string when receiving data from the network when they want JSON. So I don't think we'll remove it.

Fair enough, there will be a hit in this case. However, I'd argue that passing data directly from the wire to JSON is not a common case, so I'd leave this as an "advanced, you know what you're doing" API, and offer something less dangerous and more convenient for the common case.

I haven't looked at the API, but doubling it for just parse() and serialize() doesn't sound that bad, and you could probably even share all the test code?

czechboy0 commented 8 years ago

Yup, it's a reasonable request. But re your first point - Jay is used by Vapor, the leading swift Web framework, and going between network data and parsed JSON is extremely common there, so performance is definitely key here. A better error and more documentation should hopefully help here (and possibly add that String API later).

remko commented 8 years ago

You're right; it's not because many users won't use this API directly, that they won't use it a lot indirectly through lower dependencies such as Vapor.

Would you take a PR for a stringFrom or *FromString variant for methods that take [UInt8]? They would just be String() wrappers for the [UInt8] methods, so they would only need a single test each I would say.