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

fix json model problems so that you can access objects correctly #329

Merged
merged 2 commits into from
Nov 6, 2018

Conversation

thomaszurkan-optimizely
Copy link
Contributor

@thomaszurkan-optimizely thomaszurkan-optimizely commented Oct 31, 2018

The PR fixes the issue with Swift 4.2 accessing the entities in a data model and having to cast them because the protocol is not actually defined.

The problem is that these were not implemented correctly to start with. The entities should be what an array should contain. The protocols are just notations for JSONModel (OPTLYJSONModel). So, for example:

@property (nonatomic, strong, nonnull) NSArray<OPTLYExperiment> *experiments;

The above old implementation defines a array of OPTLYExperiment protocol objects. However, the OPTLYExperiment contains no methods or properties. The proper declaring is the following:

@property (nonatomic, strong, nonnull) NSArray<OPTLYExperiment*><OPTLYExperiment> *experiments;

This seems to be a JSONModel specific annotation via the protocols at the end.

**We should do this for all data model objects using arrays of protocols.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.579% when pulling a72b3d4 on fixProjectConfigTypes into 259ece0 on master.

@coveralls
Copy link

coveralls commented Oct 31, 2018

Coverage Status

Coverage decreased (-62.4%) to 35.129% when pulling a72b3d4 on fixProjectConfigTypes into 259ece0 on master.

Copy link
Contributor

@mikeproeng37 mikeproeng37 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm


if let experiments = optimizelyClient?.optimizely?.config?.experiments {
for experiment in experiments {
print(experiment.experimentKey)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Print? Is this desired?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's the demo app. this is just proving that the change works.

@thomaszurkan-optimizely thomaszurkan-optimizely merged commit c9211fb into master Nov 6, 2018
thomaszurkan-optimizely added a commit that referenced this pull request Nov 8, 2018
* fix json model problems so that you can access objects correctly

* use the project config to correctly get experiments
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.

4 participants