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

Adds a @LossyOptional property wrapper #30

Merged
merged 5 commits into from
Dec 22, 2020

Conversation

moyerr
Copy link
Contributor

@moyerr moyerr commented Dec 8, 2020

When decoding an Optional type, it is still possible for decoding to fail and throw an error, rather than falling back to a nil value. This PR addresses that issue by adding a @LossyOptional property wrapper, which is just a typealias of @DefaultCodable using a new DefaultNilStrategy. Possible README usage description below:

@LossyOptional

Optional codable values can be frustrating in that, if decoding fails for any reason, we are often given an error instead of a nil value, like we might expect.

@LossyOptional fixes this annoyance by defaulting decoded Optionals to nil if the Decoder is unable to decode the value.

Usage

struct Image: Codable {
    @LossyOptional var url: URL?
}

let json = #"{ "url": "https://website .test" }"#.data(using: .utf8)!
let result = try JSONDecoder().decode(Image.self, from: json)

print(result.url) // nil

Without the @LossyOptional property wrapper here, we would get a DecodingError.dataCorrupted(_:) error instead.

@marksands
Copy link
Owner

I don't think @DefaultNil is a good name here, since this is really providing a catch handler for a failed decoding. I haven't come up with a good alternative though. @SetNilOnError, @FallbackToNil, @FailingToNil, etc. 🤔

@moyerr
Copy link
Contributor Author

moyerr commented Dec 8, 2020

@marksands Okay yeah, I see what you mean. I like @FallbackToNil or @FailsToNil. Or what do you think about following the Lossy naming pattern and going with @LossyOptional? Whatever your preference, I'm happy to make the change 😄

@marksands
Copy link
Owner

marksands commented Dec 14, 2020

I like LossyOptional. Let's run with that. That gives me some ideas to make some of my other implementations more generic and flexible. 🙂

It also might be worth adding a test that demonstrates the Lossy problem outside of URL types:

	func testDecodingWithBadIntegerConversion() throws {
		struct Fixture: Codable {
			@LossyOptional var a: Int?
			@LossyOptional var b: Int?
		}

		let jsonData = #"{ "a": 3.14, "b": 3 }"#.data(using: .utf8)!
		let fixture = try JSONDecoder().decode(Fixture.self, from: jsonData)
		XCTAssertNil(fixture.a)
		XCTAssertEqual(fixture.b, 3)
	}

Copy link
Owner

@marksands marksands left a comment

Choose a reason for hiding this comment

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

See also the comment I left about adding an additional fixture with another type(s).

Tests/BetterCodableTests/DefaultNilTests.swift Outdated Show resolved Hide resolved
@moyerr moyerr changed the title Adds a @DefaultNil property wrapper Adds a @LossyOptional property wrapper Dec 16, 2020
@moyerr
Copy link
Contributor Author

moyerr commented Dec 16, 2020

@marksands Thanks for the feedback! Changes addressed, and original PR description updated with name change 😃

@marksands marksands merged commit 1b51bf9 into marksands:master Dec 22, 2020
@marksands
Copy link
Owner

Thank you! 🙇

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.

None yet

2 participants