Skip to content
This repository has been archived by the owner on Dec 20, 2023. It is now read-only.

Feature/optional decodable #38

Merged
merged 9 commits into from
Dec 13, 2017
Merged

Conversation

abretagne-axt
Copy link
Contributor

  • Add the attribute JSONIgnored in the userInfo xcdatamodel
    -- Allow to avoid decodable parsing generation for specific attributes
  • Handle optional parsing try? with relationship and other objects like Date

@abretagne-axt
Copy link
Contributor Author

Just fixed Date parsing (my custom template was using a custom DateFormatter).
Switch it back to Date.decode()

Copy link
Collaborator

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

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

Side questions:

  1. I'm surprised there were only one unit test to change and not more 😉 I hope we cover all the possible cases

  2. For the end user, I wonder what's best:

  • To only decode attributes that have their JSONKey user-info set, and ignore all the attributes without that key
  • Or to decode all attributes (and if they have no JSONKey, infer that JSONKey to be the same name as the attribute itself) and require a JSONIgnored to explicitly exclude keys

My PoV is:

  • Using the new JSONIgnored has the advantage of not having to repeat the JSONKeyPath if 80% of your keys have the same name as your attribute. But that feels like adding a little complexity in the logic / rules and understanding for users of gyro.
  • OTOH, only decoding attributes which have JSONKeyPath explicitly set is more easy to understand as a global logic (and explain in the documentation), but requires to set the JSONKeyPath also for keys that have the same JSON key and attribute name

Open for discussion

@@ -34,3 +34,6 @@ extension Shop: Decodable {
}
}
```
# Specific JSON tasks
Copy link
Collaborator

Choose a reason for hiding this comment

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

Depending on your markdown interpreter, some Markdown renderers are a little strict and expect an empty line after section titles, otherwise they won't render it as titles. So better add empty newlines after those ;)

@abretagne-axt
Copy link
Contributor Author

As discussed, for now we mostly have the case to parse all the attributes contained in our models. The specific need looks more to be to ignore a specific property during parsing, detected by checking the JSONIgnored key's presence.

I think it's more efficient this way, even if the usage could be a little bit more complex to understand ;)

@AliSoftware
Copy link
Collaborator

Ok, fair enough 😉

@AliSoftware AliSoftware merged commit 8c7fcae into master Dec 13, 2017
@AliSoftware AliSoftware deleted the feature/optional_decodable branch December 13, 2017 15:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants