chadaustin / sajson

Lightweight, extremely high-performance JSON parser for C++11
MIT License
568 stars 43 forks source link

How to use Swift version? #38

Open own2pwn opened 7 years ago

own2pwn commented 7 years ago

Hey there! I think switching enum with guard case . for every object in json isn't the best way, am I right? Could you show me real world (production) examples of using this lib?

chadaustin commented 7 years ago

Hi! Unfortunately I don't have access to any public code that uses the library, but maybe @aeidelson could sanitize and share some.

guard case sounds fine to me though - do you have a specific objection? How else would you handle the case that the value is something you didn't expect?

If you're asking about overall structure, usually you write a series of functions from a JSON value to an Optional<SomeStruct> and compose them appropriately to decode the entire document.

Does that help?

aeidelson commented 7 years ago

Yea, so as a convenience we (privately) declared a number of helper getters to read fields of various expected types. For example, String is declared as:

extension sajson_swift.ValueReader {
    public var stringValue: String {
        if case .string(let v) = self {
            return v
        }
        return ""
    }

    public var string: String? {
        if case .string(let v) = self {
            return v
        }
        return nil
    }
}
...
let s = someValue.string

We haven't merged these back because many of them aren't super well thought out and are pretty specific to our situation (keeping backwards compatibility with the library we moved from). But I think it would be totally reasonable to extend similar getters in SaJSON as a convenience.

aeidelson commented 7 years ago

I'm happy to take that on if you want @chadaustin? And we could probably add some better documentation around how to use the Swift wrapper

chadaustin commented 7 years ago

@aeidelson If you'd like, feel free! @own2pwn do you have any input into how you'd prefer decoder functions be written?

t089 commented 7 years ago

A while back I began on implementing a JSON Decoder that uses Swift4‘s ˋDecodableˋ protocol and internally uses Sajson. It’s not finished yet, but ultimately that’s the way to go I guess... you can check it out in my fork here: https://github.com/t089/sajson/blob/master/swift/sajsonTests/FastJSONDecoder_Tests.swift

own2pwn commented 7 years ago

Hey, I've implemented it like aeidelson have. But that leads to deal with optional values (because of Swift's dictionary subscript). E.g.

let items = json.array
for item in items {
  let item = item.object // item's type is: [String: Value]
  let id = item["id"]?.stringValue // here we deal with String?
}

Using if case all fine if we talk about optionals but not for readability, IMO.

if case .array(let items) = json {
  var it = items.makeIterator()
  while let elem = it.next() {
    if case .object(let item) = elem {
      let id = item["id"].stringValue // because of overriding sajson's subscript, we get String -- that's nice
    }
  }
}

It would be very nice if one could use it swiftyJson way: like aeidelson suggestion with computed properties but one that returns optional and one that return parsed value or something default (string and stringValue).

Maybe there is a way to use ASTNode directly w/o enums?

own2pwn commented 7 years ago

Also, overriding sajson's subscript leads to dirty way of handling non existing keys in json: we should return malformed ASTNode or we throw preconditionFailure

chadaustin commented 7 years ago

Thought about it a bit: I like the idea of extending to have getters something like:

extension sajson_swift.Value {
    var string: String?
    func string(withDefault: String) -> String
    // and similar for Boolean, Arrays, etc.
}

Perhaps even getters that throw?

Anyone want to submit a PR?

I don't know anything about the new Decodable protocol, so if there's consensus around @t089's fork I'd be happy to merge that too. :)

t089 commented 7 years ago

It‘s not in a mergeble(?!?) state yet. I’ll raise a PR once it is. Regardless, more convenient accessors are a plus and complement the Decodable conformance nicely.

Am 04.10.2017 um 02:08 schrieb Chad Austin notifications@github.com:

Thought about it a bit: I like the idea of extending to have getters something like:

extension sajson_swift.Value { var string: String? func string(withDefault: String) -> String // and similar for Boolean, Arrays, etc. } Perhaps even getters that throw?

Anyone want to submit a PR?

I don't know anything about the new Decodable protocol, so if there's consensus around @t089's fork I'd be happy to merge that too. :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

own2pwn commented 7 years ago

@chadaustin wouldn't it be better to prefer optionals rather than throwing? If I recall correctly Apple recommends using throwing funcs when dealing with hardware or when we want to describe error in more detail.

I'm happy with idea of non-optional subscript that return default value if extract failed along with two getters for other types (optional getter and getter with default value)