WIP: or combinator improvement
Attempt to fix #141
I like that this patch gets rid of UnibaseDecoder all together which simplifies things a bit. However I do not like silly composed getter it introduces just so that array reduce will not get confused by the fact that argument is either Decoder<B, P> or CompositeDecoder<P>.
I think it would be better to either
- Accept the fact that TS inference is going to be poor and compose things statically (as in manually listing all operands
base32.decoder.or(base64.decoder).or(base58.decoder).... Not ideal but I'm guessing number of decoders isn't going to be really big anyway. - Export another function like
export const fromTable = (decoders) => new CompositeDecoder(decoders)so that one could simply provide table when needed as opposed to building up decoder withoroperator. P.S.: I originally left this out, becauseorworks with composed and single base decoders, table on the other hand requires only single base decoders.
silly
composedgetter it introduces
Yeah, that is pretty silly. Although maybe the clean-up in there is worth it, it's quite nice now, aside from that.
I don't mind your fromTable idea either, I reckon that's going to cover the common use-cases of this. And we could copy the pattern if/when we get around to doing codecs because we have js-ipfs doing its own code->codec mapping in multiple places that would be nice to try and simplify with a single call through a composed stack. Your ipnft thing could probably do with it too. Mostly you just want to say "I have some encoders, squish them together so I have one thing please" and having to chain .or() is going to lead to the typical reduce() pattern to make it scale cleanly.
"I have some encoders, squish them together so I have one thing please" and having to chain
.or()is going to lead to the typicalreduce()pattern to make it scale cleanly.
I don't know if it's not the long list of things doing .or(a).or(b).or(c)... isn't really that cumbersome IMO, but yeah supporting long lists in some way seems like a good idea.
I think I'll make few more changes to this pr specifically:
- Get rid of
.composedgetter this introduces. - Introduce
fromTablefunction that take care of composing things. - Introduce some function that turns
Decoder<B, P>->CompositeDecoder<P>so it could be used if needed in place of silly getter. Mostly so thatfromTablecould be implemented.
@Gozala : what are the next steps here? Is this work being carried forward?
(question from triage) @Gozala would it be less disruptive if we update the interface instead?
(question from triage) @Gozala would it be less disruptive if we update the interface instead?
I’m not sure how would you do that without going and changing an implementation to comply with it, unless you ts-ignore it away.
@Gozala : what are the next steps here? Is this work being carried forward?
Sorry I missed that comment. We can either take this as is which is better that what we have and addresses the issue at hand, or I can try to find time to make changes discussed. Those changes could be future improvements however as they would just add more functions.