Skip to content

Added init(jsonString) and made init(data) throw#731

Closed
ldiqual wants to merge 2 commits intomasterfrom
feature/initJsonString
Closed

Added init(jsonString) and made init(data) throw#731
ldiqual wants to merge 2 commits intomasterfrom
feature/initJsonString

Conversation

@ldiqual
Copy link
Copy Markdown
Contributor

@ldiqual ldiqual commented Nov 23, 2016

@zhigang1992 @wongzigii

Fixes #459 and #456

This is an implementation of my proposal in #459 (comment). At a high level the changes are as follow:

  • Added init(jsonString: String) throws which creates a JSON object from a valid json string. If the string cannot be converted to data or parsed by NSJSONSerialization, the initializer will throw an appropriate error as opposed to failing silently.
  • Added documentation + tests around the new initializer and updated the README
  • Removed JSON.parse which was doing the same thing (but would fail silently)
  • Made init(data: Data) throw as well, to handle invalid JSON data coming through.

This is a code breaking change since init(data: Data) now throws.

Let me know what you think.

Comment thread Source/SwiftyJSON.swift
- returns: The created JSON
*/
public init(data:Data, options opt: JSONSerialization.ReadingOptions = .allowFragments, error: NSErrorPointer = nil) {
public init(data: Data, options: JSONSerialization.ReadingOptions = .allowFragments) throws {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1 for throws here.

Comment thread Source/SwiftyJSON.swift
/// - Throws: Throws if the string cannot be converted to raw data, or if it is not a valid JSON string
public init(jsonString: String, encoding: String.Encoding = .utf8, options: JSONSerialization.ReadingOptions = .allowFragments) throws {
guard let data = jsonString.data(using: encoding) else {
throw NSError(domain: ErrorDomain, code: ErrorInvalidJSON, userInfo: [NSLocalizedDescriptionKey: "Provided string is not a valid JSON string"])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since NSError Bridging was launched in Swift 3, we could consider to move NSError to Swift's Error. Another PR for this is welcomed, though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@wongzigii Great suggestion! Let's track it here: #749

@ldiqual
Copy link
Copy Markdown
Contributor Author

ldiqual commented Dec 1, 2016

@wongzigii Are we good on this PR?

@wongzigii
Copy link
Copy Markdown
Member

wongzigii commented Dec 1, 2016

I would like to defer to @zhigang1992 and move on.

@wongzigii
Copy link
Copy Markdown
Member

Since this is a breaking change, what about adding a change log entry for this?

@ldiqual
Copy link
Copy Markdown
Contributor Author

ldiqual commented Dec 3, 2016

@wongzigii Great suggestion. Updated the CHANGELOG.

@ldiqual
Copy link
Copy Markdown
Contributor Author

ldiqual commented Dec 13, 2016

@zhigang1992 Bump

@wongzigii
Copy link
Copy Markdown
Member

@ldiqual I want to get this into mater. Could you resolve the conflict? Seems the conflict is related to #664

@wongzigii
Copy link
Copy Markdown
Member

Closing in favor of #833

@wongzigii wongzigii closed this Apr 16, 2017
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