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

Serialisation to JSON #51

Closed
matelo opened this issue May 18, 2017 · 8 comments · Fixed by #67
Closed

Serialisation to JSON #51

matelo opened this issue May 18, 2017 · 8 comments · Fixed by #67
Assignees
Milestone

Comments

@matelo
Copy link

matelo commented May 18, 2017

Is there possibility to serialise generated object to JSON? We can do initWithDictionary, but what about to have something like dictionary method, which will return dictionary to be send as JSON via REST?

@rahul-malik
Copy link
Collaborator

Adding a way to convert the object to a dictionary representation is certainly valuable. We've discussed adding this at Pinterest for a while but haven't had a strong reason to do so just yet.

We could certainly add it and I'd be happy to review a
patch if you'd like to work on it or I can take a look in a bit.

What do you think should happen if we have 'nil' properties? Store NSNull as values? What if a property hasn't been set yet?

@matelo
Copy link
Author

matelo commented May 18, 2017

yes, I started to work on my own patch already:) I am currently facing this problem with nil values.
In our current project (where I want to use it) I would expect that if value == nil then this property will not be placed into final dictionary (JSON). I don't know if this is universal solution, maybe it could be configurable by some input parameter, but for now it enough for us.

However I am facing another issue, like when I receive object from REST which contains number, Plank's generated class store it to NSInteger. But NSInteger cannot be nil (if parameter was not received), so it is set to 0. But how can I check if this value is really 0, or it was not received at all. In some cases it makes difference, for example some update service.

@rahul-malik
Copy link
Collaborator

rahul-malik commented May 18, 2017

@matelo - Sounds great. We have a struct internally ("dirtyProperties") on each model that you can utilize for detecting if a field has been set. Below is an example of how we've utilized it.

From ObjectiveCBuilderExtension.swift:

self.properties.map({ (param, _) -> String in
  ObjCIR.ifStmt("\(self.dirtyPropertiesIVarName).\(dirtyPropertyOption(propertyName: param, className: self.className))") {
     ["_\(param.snakeCaseToPropertyName()) = modelObject.\(param.snakeCaseToPropertyName());"]
  }
}).joined(separator: "\n")

Couple things I'd like to see from the implementation:

  • The method signature should be: - (NSDictionary <NSString *, NSObject *> *)dictionaryRepresentation;
  • The keys of the dictionary should match the keys in the JSON schema declaration. Basically this will allow us to test passing the result of dictionaryRepresentation to initWithDictionary and testing if the models are equivalent.
  • Properties that have not been set (or received) should be omitted from the dictionary.
  • Properties that have been set but are null/nil should be represented as [NSNull null]
  • ADT classes should return the dictionary representation of whatever value they contain internally.

Sound good? Looking forward to your patch and will help get this in.

@bkase
Copy link
Contributor

bkase commented May 18, 2017

It would be nice to have a test that for all example schemas and for a bunch of arbitrary dictionaries per schema:

dict == dictionaryRepresentation(initWithDictionary(dict))

@rahul-malik we could do this nicely with SwiftCheck https://github.com/typelift/SwiftCheck

@matelo
Copy link
Author

matelo commented May 18, 2017

@rahul-malik I totally agree with all the points, just have a question regarding the point

Properties that have not been set (or received) should be omitted from the dictionary

Do you have some hint how can I check if the value was/wasn't set, when its type is NSInteger? Because AFAIK, unset value of NSInteger is 0 - which also can be desired (set) value.

@rahul-malik
Copy link
Collaborator

@matelo - When you're checking if a property is set, don't use it's value but rather the dirtyProperties struct. You can add an if statement around adding values to the dictionary.

This snippet below loops through all properties on an object and generates an if statement for each one. That way the only times you will add anything to the dictionary will be after you've confirmed the property is set.

You'll need to replace the "ADD STUFF TO DICTIONARY" portion with specific ways to convert each schema type into something that can be inserted into a dictionary.

self.properties.map({ (param, schema) -> String in
  ObjCIR.ifStmt("\(self.dirtyPropertiesIVarName).\(dirtyPropertyOption(propertyName: param, className: self.className))") {
       // ADD STUFF TO DICTIONARY  
  }
}).joined(separator: "\n")

You should be able to reference other files to get a sense of how this is accomplished but let me know if this is confusing. Thanks!

@rahul-malik rahul-malik added this to the 1.1 milestone May 19, 2017
@matelo
Copy link
Author

matelo commented May 23, 2017

can I create branch (or can you create branch) for this enhancement? I already have done almost all the types representation to NSDictionary. Thanks

@rahul-malik
Copy link
Collaborator

I would advise creating a fork of the repository and applying your changes on a branch there. Then you'll be able to create a pull request I can look at and provide feedback while you iterate.

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

Successfully merging a pull request may close this issue.

3 participants