Skip to content
This repository has been archived by the owner on Feb 8, 2021. It is now read-only.

Add valueForKeyPathCoding #98

Closed
wants to merge 3 commits into from
Closed

Add valueForKeyPathCoding #98

wants to merge 3 commits into from

Conversation

rahul0x24
Copy link
Contributor

I have not tested this much but this will allow us to use keyPaths when needed.

struct Person: Glossy {

    let id: String?
    let name: String?

    // MARK: - Deserialization

    init?(json: JSON) {
        self.id = "args.id" <~~ json
        self.name = "args.name" <~~ json
    }

    // MARK: - Serialization

    func toJSON() -> JSON? {
        return jsonify([
            "args.id" ~~> self.id,
            "args.login" ~~> self.name
            ])
    }

}

@rahul0x24
Copy link
Contributor Author

The toJSON() is not working correctly right now. If you are interested in this feature then i can spend more time optimising it and include test cases.

@hkellaway
Copy link
Owner

i'm not quite gathering the utility of this. what is not working about toJSON() that this fixes?

@rahul0x24
Copy link
Contributor Author

Value for key path increases the flexibility. If we have a JSON like below.

{
    "addressLine" : "",
    "city" : "",
    "state" : "",
    "location": {
        "lat" : 23.0078201,
        "lng" : 88.5428696
    }
}

then we can skip the Location struct and use location in directory Address object using keyPath

struct Address: Decodable {

    let addressLine: String?
    let city: String?
    let state: String?
    let latitude: Double?
    let longitude: Double?

    // MARK: - Deserialization

    init?(json: JSON) {
        self.addressLine = "addressLine" <~~ json
        self.city = "city" <~~ json
        self.state = "state" <~~ json
        self.latitude = "location.lat" <~~ json
        self.longitude = "location.lng" <~~ json
    }

}

This pull request is working fine in Deserialisation but not while converting back to JSON().

For example you can run the test case at https://github.com/RahulKatariya/Reactofire/blob/master/ReactofireTests/ReactofireTests.swift

@hkellaway
Copy link
Owner

i see. does this work for nested models? e.g. i'd implement the example as below - does decoding still work? what about if i wanted a model with mixed properties - i.e. some are other Glossy models and some are declared in the format you're proposing (e.g. args.id).

i do appreciate the thought/work, but i'd not consider it unless it worked for toJSON() and the cases mentioned above.

struct Person: Glossy {

    let args: Args?

    // MARK: - Deserialization
    init?(json: JSON) {
        self.args = "args" <~~ json
    }

    // MARK: - Serialization

    func toJSON() -> JSON? {
        return jsonify([
            "args" ~~> self.args
            ])
    }

}

struct Args: Glossy {

    let id: String?
    let name: String?

    init?(json: JSON) {
        self.id = "id" <~~ json
        self.name = "name" <~~ json
    }

    func toJSON() -> JSON? {
        return jsonify([
            "args.id" ~~> self.id,
            "args.login" ~~> self.name
            ])
    }
}

@rahul0x24
Copy link
Contributor Author

Hi Harlan,

I have fixed the toJSON() method and have also added the test case which you can see at https://github.com/RahulKatariya/Gloss/blob/glossy-keypath/Sources/GlossTests/KeyPathTests.swift

Best Regards,
Rahul

@hkellaway
Copy link
Owner

great - i'll have to spend some time playing with this myself this weekend; i'll respond back here if i encounter issues

@rahul0x24
Copy link
Contributor Author

👍🏼

@hkellaway
Copy link
Owner

@RahulKatariya can you rebase this? conflicts with the latest develop

@hkellaway
Copy link
Owner

@RahulKatariya i rebased and merged this into develop outside of this PR - will be available in 0.7.0. thanks! 🎉

@hkellaway hkellaway added this to the 0.7.0 milestone Feb 6, 2016
@hkellaway hkellaway closed this Feb 6, 2016
@rahul0x24
Copy link
Contributor Author

@hkellaway Sorry i was on weekend vacation. I have one more pull request to make which i think could be available in 0.7.0. There is some issue in modelsArrayFromJson.

@hkellaway
Copy link
Owner

no prob. would it be possible to make that today? I have time to review

What is the issue?

@rahul0x24
Copy link
Contributor Author

I have opened the issue #106 .
I am trying to work on it. Hopefully i would be able to make it today.

@hkellaway
Copy link
Owner

thanks @RahulKatariya

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

Successfully merging this pull request may close these issues.

None yet

2 participants