Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make get public in JSONDecoder #19

Closed
wants to merge 1 commit into from

Conversation

raylillywhite
Copy link

This allows for JSONDecoder extensions that allow the type system to better aid in decoding. For example, you could do:

extension JSONDecoder {
    public func decode(key: String) throws -> NSURL {
        return decode(key, JSONTransformers.StringToNSURL)
    }
}

then you only need to do:

try url = decoder.decode("url")

instead of

try url = decoder.decode("url", JSONTransformers.StringToNSURL)

And with the proper extensions setup, you never need to specify how to decode something at the call site unless there are multiple ways to decode a given type. As an example of a transformer that isn't built-in that we'd like this functionality for, we have a custom ID type that is convertible to/from a string, but don't want to have to manually specify the transformer each time we're decoding an ID.

This allows for JSONDecoder extensions that allow the type system to better aid in decoding. For example, you could do:

```swift
extension JSONDecoder {
    public func decode(key: String) throws -> NSURL {
        return decode(key, JSONTransformers.StringToNSURL)
    }
}
```

then you only need to do:

```
try url = decoder.decode("url")
```

instead of

```
try url = decoder.decode("url", JSONTransformers.StringToNSURL)
```

And with the proper extensions setup, you never need to specify _how_ to decode something at the call site unless there are multiple ways to decode a given type.
@raylillywhite raylillywhite changed the title Make get and object public in JSONDecoder Make get public in JSONDecoder Dec 9, 2015
@raylillywhite
Copy link
Author

@matthewcheok great point — I had modified object to be public when I was trying to accomplish this another way, and hadn't realized that I didn't need it anymore. I've updated the pull request to only make get public.

And I realized that I gave a poor example, which could be accomplished even without this change. A better example of where this helps is a case where you have a generic struct/class that can't conform to JSONCodable unless it's type parameters do. Something along the lines of:

struct Edge<T> {
    var node: T
    var cursor: String
}

extension JSONDecoder {
    func decode<T: JSONDecodable>(key: String) throws -> [Edge<T>] {
        guard let value = get(key) else {
            return []
        }
        guard let array = value as? [JSONObject] else {
            throw JSONDecodableError.ArrayTypeExpectedError(key: key, elementType: value.dynamicType)
        }
        return try array.map { object in
            let edgeDecoder = JSONDecoder(object: object)
            return Edge(
                node: try edgeDecoder.decode("node"),
                cursor: try edgeDecoder.decode("cursor")
            )
        }
    }
}

@matthewcheok
Copy link
Owner

I wonder if there are better ways to handle this case. You're right that in this case the library cannot instantiate Edge<T> on your behalf because it does not know what T is.

It does look like your root object won't be Edge<T> or maybe even [Edge<T>] and you can write a JSONTransformer for the property containing [Edge<T>].

Also there might be a chance your generic type can conform to JSONCodable if you provide a JSONTransformer for node?

@raylillywhite
Copy link
Author

Ahh, thanks for pointing me in the right direction. I can get the benefit of the get method's functionality by using a JSONTransformer. It hadn't occurred to me to use a JSONTransformer for complex objects, but that accomplishes what I want here.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants