Skip to content

Fix decoding error for searchRecentTweets if there is no data - #54

Open
Tunous wants to merge 2 commits into
daneden:mainfrom
Tunous:TwitterAPIDataIncludesAndMeta-nil-data
Open

Fix decoding error for searchRecentTweets if there is no data#54
Tunous wants to merge 2 commits into
daneden:mainfrom
Tunous:TwitterAPIDataIncludesAndMeta-nil-data

Conversation

@Tunous

@Tunous Tunous commented Oct 2, 2022

Copy link
Copy Markdown
Contributor

When calling searchRecentTweets method the response from Twitter might not contain data property which results in decoding error for TwitterAPIDataIncludesAndMeta.

Example response:

{
  "meta": {
    "result_count": 0
  }
}

This pull request makes data property optional which resolves decoding issue. But perhaps a better idea would be to introduce a separate type for cases where data is an Array and instead of optional return empty? Or even change Resource type of TwitterAPIDataIncludesAndMeta to always expect array? Looking at the tests there are no cases where data is of non-array type.

@daneden

daneden commented Oct 3, 2022

Copy link
Copy Markdown
Owner

Oh this is an odd one; the reason you won't find any tests with TwitterAPIDataIncludesAndMeta where Resource isn't an array is that meta typically indicates paginated responses. Search results are no exception, but it’s interesting that Twitter returns an object with no data field—one might expect the following instead:

{
  "data": [],
  "meta": {
    "result_count": 0
  }
}

I like your suggestion of changing Resource for TwitterAPIDataIncludesAndMeta to always expect an array e.g. <Resource: [Codable]> and think that might be better than having data be optional.

@Tunous

Tunous commented Oct 3, 2022

Copy link
Copy Markdown
Contributor Author

I've changed my code to make data a required array of Resource. I think that should be enough.

I tried to make Resource a Sequence but then I'm not really sure how to create an empty sequence in added initializer.

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.

2 participants