tcomb icon indicating copy to clipboard operation
tcomb copied to clipboard

Code duplication within `union` and `dispatch`

Open ArmorDarks opened this issue 8 years ago • 4 comments

Version

3.2.23

The issue

Let's take an example:

const Price = t.union([t.Number, t.struct({
  from: t.Boolean,
  value: t.Number
}, { name: 'PriceObject', strict: true }))

Looks clean and elegant. But to use that union, we have to declare a dispatch method. And here comes the issue:

Price.dispatch = (x) => t.Number.is(x)
  ? t.Number // hm, that feels like a duplication of `union` content
  : // eh, what have to be here? We don't have any reference to pass on

To resolve that issue, we need to declare struct as a standalone variable:

const PriceObject = t.struct({
  from: t.Boolean,
  value: t.Number
}, { name: 'PriceObject', strict: true })

const Price = t.union([t.Number, PriceObject])
Price.dispatch = (x) => t.Number.is(x)
  ? t.Number
  : PriceObject

And suddenly that doesn't look that elegant anymore. More of it, it still involves nasty code duplication:

sublime_text_2017-09-13_00-13-37

Which boils down to that point that union doesn't solve that task as good as it would be expected to.

Proposals

To pass types to dispatch?

const Price = t.union([t.Number, PriceObject])
Price.dispatch = (x, types) => t.Number.is(x)
  ? types[0]
  : types[1]

Doesn't look reliable, but in some cases might be better alternative

However, I have a feeling that it should look like this instead, where dispatch defines our "custom" union at first place, thus we don't need to declare relatively close thing twice:

const Price = t.union((x) => t.Number.is(x)
  ? t.Number
  : PriceObject
)

It almost look like we need to use here refinement or irreducible:

const Price = t.irreducible('Price', (x) => t.Number.is(x)
  ? t.Number(x) && true
  : PriceObject(x) && true
)

However, it will make validation errors less informative. Besides, with such approach, t.validation() will stop on first error of irreducible and will not validate further.

Instead of this (for union with large object, which has multiple Offer entries with errors):

Invalid value "nope" supplied to /offers/0/1/price/1/value: Number
Invalid value "nopeAgain" supplied to /offers/1/0/price/1/value: Number

It will return only:

TypeError: [tcomb] Invalid value "nope" supplied to PriceObject/value: Number

ArmorDarks avatar Sep 12 '17 21:09 ArmorDarks

const Price = t.union((x) => t.Number.is(x)
  ? t.Number
  : PriceObject
)

Unions like that are not introspectable though

Price.meta.types // ???

Admittedly structs are a bit awkward when used in unions, perhaps you could just use interfaces: you don't even need to define a dispatch function

const Price = t.union([t.Number, t.interface({
  from: t.Boolean,
  value: t.Number
}, { name: 'PriceObject', strict: true })])

const x = Price(1) // ok
const y = Price({from: true, value: 2}) // ok

gcanti avatar Sep 13 '17 01:09 gcanti

Unions like that are not introspectable though

Yeap, good point. I realized that yesterday too. Definitely not an option then

What do you think about passing types to dispatch then?

For instance, after giving it a better thought, it seems to be quite an option, especially with destructuring:

const PriceObject = t.struct({
  from: t.Boolean,
  value: t.Number
}, { name: 'PriceObject', strict: true })

const Price = t.union([t.Number, PriceObject])

Price.dispatch = (x, [Number, PriceObject) => Number.is(x)
  ? Number
  : PriceObject

It might seem like we have here even more code duplication, but it is different. In original code we had to pass same types few times and they don't have any real relation to union declaration itself:

const PriceObject = t.struct({
  from: t.Boolean,
  value: t.Number
}, { name: 'PriceObject', strict: true })

const Price = t.union([t.Number, PriceObject])

Price.dispatch = (x) => t.Number.is(x) // I can't make here check against one of declared within union types, instead I'm making a wild guess, and if union will change, this part won't reflect change
  ? t.Number // this is completely new, and doesn't relate to union itself directly
  : PriceObject // this is a thing from outside world, it will silently break if I will change second argument in union

In case of types passing to dispatch and destructuring we're reusing already declared types within union and tied to them.

Besides, it will allow to avoid declaring types within variables just to pass them to union and dispatch:

const Price = t.union([t.Number, t.struct({
  from: t.Boolean,
  value: t.Number
}, { name: 'PriceObject', strict: true })])

Price.dispatch = (x, [Number, PriceObject) => Number.is(x)
  ? Number
  : PriceObject

Admittedly structs are a bit awkward when used in unions, perhaps you could just use interfaces: you don't even need to define a dispatch function

Ah, thanks for the hint! After reading API docs I didn't realize that interface is different from struct. I thought that it is kind of legacy thing. I guess it worth to add a note, that it is better suited for pure data structures and can be used in union that way.

That completely resolves particularly my issue, but my proposal above remains actual, since it will allow to express better relation between union and dispatch.

ArmorDarks avatar Sep 13 '17 13:09 ArmorDarks

What do you think about passing types to dispatch then?

Looks good to me, would you like to send a PR?

gcanti avatar Sep 13 '17 13:09 gcanti

Hey, What about wrapping it in a helper class with the Builder Pattern For example:

class UnionBuilder {
  constructor(t){
    this.t = t
    this.structs = []
    this.union = null
    this.name = null
  }
  withStruct(...args){
    this.structs.push(this.t.struct(...args))
    return this
  }
  withName(name){
    this.name = name
    return this
  }
  withDispatch(fn){
    this.dispatch = fn
    return this
  }
  build(){
    return this.t.union(this.structs, this.name).dispatch = this.dispatch(this.structs)
  }
}

let ExampleUnion = new UnionBuilder(t)
.withStruct({ step1: t.Number })
.withStruct({ step2: t.Number })
.withName('Example')
.withDispatch(function ([ type1, type2 ]){
  return function (x) {
    return x.type === "type1" ? type1 : type2
  }
})
.build()

console.log(ExampleUnion({type: "type1", n: 2}))

This way you get your desired functionality + you dont need to mess with internal tcomb code. Dont forget to add validations, I dropped them for the sake of the example.

yuraxdrumz avatar Oct 11 '19 11:10 yuraxdrumz