Skip to content

Commit

Permalink
Remove optional decoding as an explicit operation
Browse files Browse the repository at this point in the history
Previously, we've used a special operator (`<|?`) for denoting that we were
decoding to an optional property. This operator had special behavior that
meant it would _only_ fail if the decoding operation itself failed, but would
still succeed if the key was missing.

I think that ultimately, this was a confusing distinction. Although it was
nice and concise, it made it difficult to reason about some transformations
(like flatMapping into a secondary transformation), and it also meant we
needed to double the surface area of the API. The special behavior around
_when_ we failed the decoding was also potentially confusing and could lead to
unexpected results for people that weren't completely familiar with the
distinction between how we handled various decoding failures internally.

Additionally, we've had another solution to this problem for nearly the entire
time this library has existed: `Decoded.optional`. This is a _much_ simpler
transformation, and it simply converts the `Decoded` to an `Optional` and then
wraps it in `.success`.

It seems reasonable, then (especially with the additional API changes
currently in flight) to reduce the surface area _and_ the complexity of our
API at the same time by removing the `<|?` operators in favor of explicitly
wrapping our extraction operations in a call to `.optional`.

Note that this _also_ means we can use conditional conformance to make conform
`Optional` conform to `Decoded` directly.
  • Loading branch information
gfontenot committed Apr 27, 2018
1 parent 36f3741 commit 56ecd1a
Show file tree
Hide file tree
Showing 8 changed files with 13 additions and 66 deletions.
1 change: 0 additions & 1 deletion Sources/Argo/Operators/Argo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,3 @@ precedencegroup ArgoDecodePrecedence {
}

infix operator <| : ArgoDecodePrecedence
infix operator <|? : ArgoDecodePrecedence
42 changes: 0 additions & 42 deletions Sources/Argo/Operators/DecodeOperators.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,25 +17,6 @@ public func <| <A: Decodable>(json: JSON, key: String) -> Decoded<A> where A ==
return json <| [key]
}

/**
Attempt to decode an optional value at the specified key into the requested
type.

This operator is used to decode an optional value from the `JSON`. If the key
isn't present in the `JSON`, this will still return `.Success`. However, if
the key exists but the object assigned to that key is unable to be decoded
into the requested type, this will return `.Failure`.

- parameter json: The `JSON` object containing the key
- parameter key: The key for the object to decode

- returns: A `Decoded` optional value representing the success or failure of
the decode operation
*/
public func <|? <A: Decodable>(json: JSON, key: String) -> Decoded<A?> where A == A.DecodedType {
return json <|? [key]
}

/**
Attempt to decode a value at the specified key path into the requested type.

Expand All @@ -53,26 +34,3 @@ public func <|? <A: Decodable>(json: JSON, key: String) -> Decoded<A?> where A =
public func <| <A: Decodable>(json: JSON, keys: [String]) -> Decoded<A> where A == A.DecodedType {
return flatReduce(keys, initial: json, combine: decodedJSON) >>- A.decode
}

/**
Attempt to decode an optional value at the specified key path into the
requested type.

This operator is used to decode an optional value from the `JSON`. If any of
the keys in the key path aren't present in the `JSON`, this will still return
`.Success`. However, if the key path exists but the object assigned to the
final key is unable to be decoded into the requested type, this will return
`.Failure`.

- parameter json: The `JSON` object containing the key
- parameter keys: The key path for the object to decode, represented by an
array of strings
- returns: A `Decoded` optional value representing the success or failure of
the decode operation
*/
public func <|? <A: Decodable>(json: JSON, keys: [String]) -> Decoded<A?> where A == A.DecodedType {
switch flatReduce(keys, initial: json, combine: decodedJSON) {
case .failure: return .success(.none)
case .success(let x): return A.decode(x) >>- { .success(.some($0)) }
}
}
4 changes: 2 additions & 2 deletions Sources/Argo/Types/StandardTypes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ extension Bool: Decodable {
}
}

public extension Optional where Wrapped: Decodable, Wrapped == Wrapped.DecodedType {
extension Optional: Decodable where Wrapped: Decodable, Wrapped == Wrapped.DecodedType {
/**
Decode `JSON` into an `Optional<Wrapped>` value where `Wrapped` is `Decodable`.

Expand All @@ -172,7 +172,7 @@ public extension Optional where Wrapped: Decodable, Wrapped == Wrapped.DecodedTy

- returns: A decoded optional `Wrapped` value
*/
static func decode(_ json: JSON) -> Decoded<Wrapped?> {
public static func decode(_ json: JSON) -> Decoded<Optional> {
return Wrapped.decode(json) >>- { .success(.some($0)) }
}
}
Expand Down
2 changes: 1 addition & 1 deletion Tests/ArgoTests/Models/Post.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ extension LocationPost: Argo.Decodable {
<*> json <| "text"
<*> json <| "author"
<*> json <| "comments"
<*> json <|? "location"
<*> .optional(json <| "location")
}
}

Expand Down
8 changes: 4 additions & 4 deletions Tests/ArgoTests/Models/TestModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ extension TestModel: Argo.Decodable {
<*> json <| ["user_opt", "name"]
<*> json <| "bool"
<*> json <| "string_array"
<*> json <|? "string_array_opt"
<*> .optional(json <| "string_array_opt")
<*> json <| ["embedded", "string_array"]
<*> json <|? ["embedded", "string_array_opt"]
<*> json <|? "user_opt"
<*> .optional(json <| ["embedded", "string_array_opt"])
<*> .optional(json <| "user_opt")
<*> json <| "dict"
}
}
Expand All @@ -50,7 +50,7 @@ extension TestModelNumerics: Argo.Decodable {
<*> json <| "int64_string"
<*> json <| "double"
<*> json <| "float"
<*> json <|? "int_opt"
<*> .optional(json <| "int_opt")

return f
<*> json <| "uint"
Expand Down
2 changes: 1 addition & 1 deletion Tests/ArgoTests/Models/User.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ extension User: Argo.Decodable {
return curry(self.init)
<^> json <| "id"
<*> (json <| ["userinfo", "name"] <|> json <| "name")
<*> json <|? "email"
<*> .optional(json <| "email")
}
}
14 changes: 2 additions & 12 deletions Tests/ArgoTests/Tests/DecodedTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,19 +78,9 @@ class DecodedTests: XCTestCase {
"email": 1
])

let expected: [DecodeError] = [
.missingKey("name"),
.typeMismatch(expected: "String", actual: String(describing: JSON.number(1)))
]

switch user {
case let .failure(.multiple(errors)):
print("expected: \(expected)")
print("actual: \(errors)")
let expected: DecodeError = .missingKey("name")

XCTAssertEqual(errors, expected)
default: XCTFail("Unexpected Case Occurred")
}
XCTAssertEqual(user.error, expected)
}

func testDecodedMaterializeSuccess() {
Expand Down
6 changes: 3 additions & 3 deletions Tests/ArgoTests/Tests/EmbeddedDecodingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ final class EmbeddedDecodingTests: XCTestCase {
}
}

func testFailOnEmbeddedObject() {
func testDecodeEmbeddedOptionalObject() {
let post: Decoded<LocationPost> = decode(json(fromFile: "bad_location_post")!)

switch post.error {
switch post.value {
case .some: XCTAssert(true)
case .none: XCTFail("Unexpected Success")
case .none: XCTFail("Unexpected Failure")
}
}
}

0 comments on commit 56ecd1a

Please sign in to comment.