thoughtbot/Argo

RFC: Renaming (or removing) the `<|` family of operators

gfontenot opened this issue · 5 comments

I'd like to think about at least renaming, or maybe even removing, the <| family of operators in Argo. I think this falls in line with the goals for the (theoretical) next release of Argo, where we're going to be renaming large parts of the lib in order to remove ambiguity or clarify library intent (see #474 and #473). There are a couple of reasons for renaming these operators:

  1. When we first designed Argo, we were unaware of the prior art of using <| for right-to-left function application in languages like F#. Since becoming aware of this, I've seen |> used more and more for left-to-right function application, and the fact that our operator conflicts with this existing expectation bothers me, and seems like it could lead to confusion.
  2. Using an operator for extracting values from JSON instances can to some fairly gnarly code that's really hard to read. Many times, this is due to ambiguous precedence between all of the different operators needed to fully implement a non-trivial decoder.
  3. The <| is incredibly specialized, and only really works with the shape JSON -> String -> Decoded<T>. This means its use is limited, and could probably be replaced by a method on JSON itself.

I'd propose making the drastic change of removing the <| family of operators in the next release of Argo in favor of a named method (JSON.extract). For the sake of discussion, here's a sample of the before/after for one of our decode methods in our test suite:

// before
extension User: Argo.Decodable {
  static func decode(_ json: JSON) -> Decoded<User> {
    return curry(self.init)
      <^> json <| "id"
      <*> (json <| ["userinfo", "name"] <|> json <| "name")
      <*> json <|? "email"
  }
}

// after

extension User: Argo.Decodable {
  static func decode(_ json: JSON) -> Decoded<User> {
    return curry(self.init)
      <^> json.extract("id")
      <*> json.extract(["userinfo", "name"]) <|> json.extract("name")
      <*> json.extractOpt("email")
  }
}

The optional method name is an interesting problem to need to solve, I'm not confident that extractOpt is the right choice there. I'd love to do extract? but that's not valid swift. Also, of note: this will be made simpler by the fact that with Swift 4.1 (which we should target for these changes) we'll be able to remove the collection operators (<||) from the lib anyway.

mdiep commented

I'm 👍 on the general idea.

How about using a subscript? That seems like it'd be very readable and obvious to everyone, no matter their experience level.

I'm not sure how to handle optionals though… Maybe a label?

extension User: Argo.Decodable {
  static func decode(_ json: JSON) -> Decoded<User> {
    return curry(self.init)
      <^> json["id"]
      <*> json["userinfo", "name"] <|> json["name"]
      <*> json[maybe: "email"]
  }
}

Or what if you got rid of the concept of <|? and always let Decoded<V?> return pure(nil) if one of the keys was missing?

I was actually talking about this at lunch and meant to come back in to propose that exact subscripting API. I was thinking subscript(optional: String) instead of maybe, but identical other than that.

Or what if you got rid of the concept of <|? and always let Decoded<V?> return pure(nil) if one of the keys was missing?

Not sure that I follow this. Are you suggesting that we push people towards using <|> in their decode functions to explicitly handle optional values? Or do you mean handling that internally as a part of Decoded? I think removing the explicit optional operator (named or symbolic) will result in ambiguity in the type system since T? is hard to disambiguate from T. I believe that's what led us to introducing the <|? operators in the first place.

mdiep commented

Or do you mean handling that internally as a part of Decoded?

This. If Optional: Decoded where Wrapped: Decoded, then can it handle getting a .null and returning nil?

Ah, sure. Conditional conformance. My brain hasn't fully groked the ways that will clean up our API yet. That passes my brain typechecker. I'd definitely be interested in trying to simplify our API as much as possible. In theory there's a benefit to explicitly marking optional properties, but I'm not sure how much the benefit outweighs the mental overhead. Probably not much.

Doing this in #488