Refactor mechanism to deserialize user-defined classes for entries #138
Conversation
loudmouth
force-pushed
the
improvement/entry-decoding
branch
from
9bf3c91 to
92d5cdf
Compare
loudmouth
force-pushed
the
improvement/entry-decoding
branch
from
92d5cdf to
271607e
Compare
| } | ||
|
|
||
| init(from decoder: Decoder) throws { | ||
| let container = try decoder.container(keyedBy: CodingKeys.self) |
There was a problem hiding this comment.
Spacing on assignment is not consistent throughout the code. For example, within this same method: https://github.com/contentful/contentful.swift/pull/138/files#diff-0312db99a9cdde535bb9947197e97a8eR148
Either have all of them aligning, or have all of them with proper whitespace. I personally prefer having them with regular whitespacing as it's simpler to read at a glance, and extra whitespace is noisy.
There was a problem hiding this comment.
i'll see what i can do! thanks @dlitvakb
There was a problem hiding this comment.
so yeah, i'm simply aligning the right hand side for all these methods to the farthest left that they can be and still remained aligned. make sense?
| var entries: [EntryDecodable] = [] | ||
| let contentTypes = decoder.userInfo[DecoderContext.contentTypesContextKey] as! [ContentTypeId: EntryDecodable.Type] | ||
|
|
||
| while entriesJSONContainer.isAtEnd == false { |
There was a problem hiding this comment.
while !entriesJSONContainer.isAtEnd {
There was a problem hiding this comment.
I actually like == false here. It's easy to miss the ! at the start and it gets especially confusing when you have a semantic variable like isAtEnd. Just my 2c.
There was a problem hiding this comment.
yeah, i've chosen as the style for the sdk to use == false always over !
There was a problem hiding this comment.
I personally prefer conciseness, but I understand that it may get lost in the syntax.
| var entries: [EntryDecodable] = [] | ||
| let contentTypes = decoder.userInfo[DecoderContext.contentTypesContextKey] as! [ContentTypeId: EntryDecodable.Type] | ||
|
|
||
| while entriesJSONContainer.isAtEnd == false { |
There was a problem hiding this comment.
Some of this code looks to be duplicated, may there be a way to de-duplicate and reuse it?
There was a problem hiding this comment.
unfortunately, these are different types and i'm just initializing a few mutable and immutable variables. i think the only refactor opportunity would be a helper that returns a tuple, but in fact the handling of data inside this json is different while iterating through the container.
| var entries: [EntryDecodable] = [] | ||
| let contentTypes = decoder.userInfo[DecoderContext.contentTypesContextKey] as! [ContentTypeId: EntryDecodable.Type] | ||
|
|
||
| while entriesJSONContainer.isAtEnd == false { |
TODO before merging:
Fixes #132
Fixes #133
Though not a direct fix for denormalizing the JSON response, the fact that the new mechanism leverages Swift 4's decodable protocol obviates the need to denormalize the json, therefore this fixes #109