Skip to content

Conversation

m-bock
Copy link
Contributor

@m-bock m-bock commented Jun 25, 2021

I think this may be useful to destructure Json data structures.

#1519

@samhh
Copy link
Contributor

samhh commented Jun 25, 2021

This is broader than this PR but is there a reason we favour pattern matching in the form of match(f, g) instead of match({ f, g })?

I've had comments from colleagues I was onboarding to fp-ts previously that match/fold is hard to understand because you don't know which callback aligns with which member of the union or sum type. It's manageable for types with fewer members like Option where you can memorise the order, but for a case like this it's going to be very difficult. In a sense, where the types overlap, we're also giving up type safety with the current approach.

Here's a comparison in terms of readability (keeping in mind you'd usually not have the type referenced in the return value!):

J.match(
  () => 'null',
  () => 'boolean',
  () => 'number',
  () => 'string',
  () => 'array',
  () => 'object'
)

J.match({
  onNull: () => 'null',
  onBoolean: () => 'boolean',
  onNumber: () => 'number',
  onString: () => 'string',
  onArray: () => 'array',
  onObject: () => 'object'
})

An additional benefit is that this lets you order the callbacks however you'd like. We could additionally experiment with how to support a universal _/otherwise fallback.

@DenisFrezzato
Copy link
Collaborator

is there a reason we favour pattern matching in the form of match(f, g) instead of match({ f, g })?

I guess for verbosity's sake. I'm fine with match(f, g) for well known members and as long as there are a couple of cases. For custom unions I usually go with an object where the keys are the name of the members. Example of usage:

type Err = { _type: 'NotFound', id: string } | { _type: 'SomeException', error: Error }

match({
  NotFound: ({ id }) => `Resource not found with ID "${id}"`,
  SomeException: ({ error }) => error.message
})

(I actually use ts-adt for pattern matching, which follows this idea).

@m-bock
Copy link
Contributor Author

m-bock commented Jun 25, 2021

This is broader than this PR but is there a reason we favour pattern matching in the form of match(f, g) instead of match({ f, g })?

Personally I'd favor the {f, g...} approach, too. I just used the positional one to stay in sync with other match functions.
In my view it's not even a matter of the number of cases, even for something simple like Either I find {left, right} safer.

_/'otherwise' cases I would not handle at this level. But I think it would make sense to add isString, isNumber (...) predicates for each case. (like it's done in other modules)

@m-bock
Copy link
Contributor Author

m-bock commented Jun 25, 2021

Btw, I guess you have noticed the as assertion needed in the last case. I tried this approach:

export const match: <Z>(
    onNull: () => Z,
    onBool: (x: boolean) => Z,
    onNum: (x: number) => Z,
    onStr: (x: string) => Z,
    onArr: (x: JsonArray) => Z,
    onObj: (x: JsonRecord) => Z
  ) => (j: Json) => Z = (onNull, onBool, onNum, onStr, onArr, onObj) => (j) =>
    j === null
      ? onNull()
      : typeof j === 'boolean'
      ? onBool(j)
      : typeof j === 'number'
      ? onNum(j)
      : typeof j === 'string'
      ? onStr(j)
      : j instanceof Array
      ? onArr(j)
      : onObj(j)

But unfortunately this did not pass the typelevel tests for TS version 3.5 :)

@m-bock m-bock requested a review from DenisFrezzato June 25, 2021 11:55
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.

3 participants