[ObjC] generating: `- (NSDictionary *) toDict;' for Objective C records

[korovkin]

Super useful to have this functionality for a few reasons:

1. easy to serialize the object to JSON or a file in order to send over HTTP or write to the file system respectively.
2. no need to comply with NSCoder complex protocol. if needed NSCoder implementation can be derived from the new toDict method very easily.
3. the code generated - (NSString *) description method can now just become: implementation to just: return [[self toDict] description]

please note, i am a complete n00b at Scala; First time see / code in Scala - any feedback would be highly appreciated.

[smarx]

Automated message from Dropbox CLA bot

@korovkin, thanks for the pull request! It looks like you haven't yet signed the Dropbox CLA. Please sign it here and update the thread so we can consider merging your code.

[korovkin]

so far have been adding the toDict manually to the generated djinni records, thus decided to code up the generator for it, so others could use it as well.

cheers !

[mknejp]

so far have been adding the toDict manually to the generated djinni records, thus decided to code up the generator for it, so others could use it as well.

You can use record extenions (add +o to your djinni record definition) so you don't have to modify the autogenerated files. You them simply create your own ABCMyRecord class deriving from the generated ABCMyRecordBase and add your custom methods to that instead. All the code Djinni generates will accept/instantiate your ABCMyRecord.

[korovkin]

@mknejp the idea is for me not to write this code by hand as it's very simple for a machine to generate this code, not in ABCMyRecordBase and not in ABCMyRecord

[korovkin]

@smarx CLA signed

[korovkin]

example of generated code:

- (NSDictionary *)toDict
{
    #define _djinni_hide_null_(_o_) ((_o_)?(_o_):([NSNull null]))

    return @{@"__class_name__": [self.class description], @"firtName": _djinni_hide_null_(self.firtName), @"lastName": _djinni_hide_null_(self.lastName), @"emails": _djinni_hide_null_(self.emails), @"gender": _djinni_hide_null_(@(self.gender))};
}

existing description function:

- (NSString *)description
{
    return [NSString stringWithFormat:@"<%@ %p firtName:%@ lastName:%@ emails:%@ gender:%@>", self.class, self, self.firtName, self.lastName, self.emails, @(self.gender)];
}

[mknejp]

It'd be also great if you ran the test suite and added some new tests for the feature.

[steipete]

As the author of the description patch +1 the idea of then just calling toDict in the description implementation. Can you add that to the PR?

[korovkin]

@steipete yes, of course. i will add that too.

pardon my poor Scala skills :)

[korovkin]

@artwyman i believe you are right, that was also the limitation of the original description method. Peter already mentioned that and is working on addressing that:

https://mobilecpp.slack.com/archives/djinni/p1446769200000039

[korovkin]

updated the PR as requested (making [description] a consumer of [toDict]) executed all the tests.

please take a look.

[korovkin]

guys, i feel like there are too many opinions here. can we please assign one reviewer, as this is introducing a pretty significant amount of pain for such a small change.

[korovkin]

some of the questions as asked and answered more than once ... as it's difficult to navigate between different opinions / requests, some of which are a bit of 'nit pickish' while some are more fundamental.

who would like to be the reviewer for this?

[artwyman]

I'll take the role of designated reviewer here. Sorry for my part of the multi-comment confusion. I'm new to the github workflow, and was disoriented by its inability to carry over comments between multiple revisions. Similarly, I think mixing discussion of the feature design with discussion of code-level nitpicks has been unproductive.

I'd like to do a bit of a reset on this pull request, since i think this feature is turning out not to be as simple as it seemed originally. Let's start with a discussion of what the feature really is, and the use case you have in mind. Understanding that affects my thoughts on the importance of processing fields in a recursive way, as well as other aspects of the feature.

What is your use case for this feature? What should other Djinni users be able to do with this feature? How might it impact other Djinni users who don't need this feature?

A few thoughts of my own on those topics:

If JSON-compatible encoding is your intent, then lack of recursion seems like a serious limitation. It might be acceptable only for your use case because you don't happen to have any nested records, but a public feature of Djinni should be more general. JSON encoding seems like either its own feature (in a toJSON method), or a specialized form of "normalized representation". There are other standard ways of normalizing data in ObjC which should be considered, such as NSCoder (as you point out) or Key Value Coding (see NSKeyValueCoding).

If the intent is simply to provide a way to access the fields of a record based on a string key rather than a name, or iterate them, then that can be accomplished with a non-recursive method. I'd personally name that method (or property?) "fieldDict" to clarify its use. Without iteration, I think that's the situation for which Key Value Coding was designed so that might be worth considering as an alternative. NSKeyValueCoding provides dictionaryWithValuesForKeys: but not any way to enumerate all keys for iteration without already knowing which keys exist, so it may not solve the whole problem.

Another thing to consider in either case is support for non-NSObject external types, which may not have a known way to convert to something which can go into an NSDictionary, in which case another new YAML field would be required, which is already leading to some heavy discussion over in #160. If you don't want to tackle that, it would be best to make this feature opt-in, so that users with existing external types don't end up with non-compiling generated code after an update.