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

Avoid '??' has non-optional type warning #207

Merged
merged 3 commits into from
Dec 3, 2019
Merged

Avoid '??' has non-optional type warning #207

merged 3 commits into from
Dec 3, 2019

Conversation

marcuswu0814
Copy link
Contributor

螢幕快照 2019-12-03 下午7 16 39

@yonaskolb
Copy link
Owner

True, something must of gotten change in a refactor. Feel free to update with a PR!

@marcuswu0814
Copy link
Contributor Author

Just forgot to run test, I will fix it ASAP. 😃

@yonaskolb
Copy link
Owner

Whoops sorry, I thought this was an issue as I viewed it through the new github notification interface. This is already a PR! :)

@marcuswu0814
Copy link
Contributor Author

螢幕快照 2019-12-03 下午8 16 21

I have no idea why the compiler say it must be unwrapped when try to build test specs. 😅

@yonaskolb
Copy link
Owner

I think it's because of the double optional, though not sure of the specifics.
Try a 2 step process

...
let jsonValue = try? JSONSerialization.jsonObject(with: jsonData),
let jsonDictionary = jsonValue as? [String: Any] else {
...

@marcuswu0814
Copy link
Contributor Author

I try to change the guard condition to:

 let jsonDictionary = (try? JSONSerialization.jsonObject(with: jsonData)) as? [String: Any] 

the error gone, any suggestion to update the template?

@yonaskolb
Copy link
Owner

Yep, the parenthesis will work too

@marcuswu0814
Copy link
Contributor Author

Which one did you prefer? I can fix that.

@yonaskolb
Copy link
Owner

I don't mind, I'll let you choose :)

@yonaskolb yonaskolb merged commit 1b6a42c into yonaskolb:master Dec 3, 2019
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