Skip to content
This repository has been archived by the owner on Feb 16, 2021. It is now read-only.

Code duplication within union and dispatch #295

Open
ArmorDarks opened this issue Sep 12, 2017 · 4 comments
Open

Code duplication within union and dispatch #295

ArmorDarks opened this issue Sep 12, 2017 · 4 comments

Comments

@ArmorDarks
Copy link

ArmorDarks commented Sep 12, 2017

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
@gcanti
Copy link
Owner

gcanti commented Sep 13, 2017

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

@ArmorDarks
Copy link
Author

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.

@gcanti
Copy link
Owner

gcanti commented Sep 13, 2017

What do you think about passing types to dispatch then?

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

@yuraxdrumz
Copy link

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants