Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cro::Transform using consumes and produces rather than type parameters #1

Closed
vendethiel opened this issue Aug 30, 2017 · 3 comments
Closed

Comments

@vendethiel
Copy link
Member

Hi,

I just wanted to know why Cro uses

class Echo does Cro::Transform {
    method consumes() { Cro::TCP::Message }
    method produces() { Cro::TCP::Message }

    method reply(Supply $source) {
       // ...
    }
}

instead of e.g.

role Cro::Transform[::From, ::To] {}
class Echo does Cro::Transform[Cro::TCP::Message, Cro::TCP::Message] {
  method reply(...) { ... }
}

When asking around, I got two separate reasons:

  • You can't correctly type a composeN (at least in Perl 6), in a case like Cro::ManyThingsInARow[A, E].new(AB, BC, CD, DE). That's solved by composing one after another, though a bit annoying.
  • Supplys aren't typed (and thus having a type parameter is less useful)

I'd like to know more about why it was done that way

@jnthn
Copy link
Member

jnthn commented Aug 30, 2017

It's interesting that this issue doesn't state a single reason why it would be better using a parametric role. :-)

Consider something like CompositeTransform. It's trivial to get the consumes and produces right for this case just by implementing the methods. Introduce a type parameter, and that (admittedly small) logic would need to be done outside of the CompositeTransform class in order to correctly fill in the role arguments. I don't think one can (successfully :)) argue that'd be a better factoring.

About the only advantage I can see in doing it with type parameters that it'd save writing out the produces/consumes methods, but it's not all that much of a saving. Arguably it'd let things like Cro::HTTP::Server say that it wants something doing Cro::Transform[Cro::HTTP::Request, Cro::HTTP::Response], but it could already do that with a where constraint, and they'll both be checked at runtime anyway, so there's no safety argument. And both would just be duplicating the checks that the composer does anyway, and so arguably give a less consistent user experience.

And that last point kinda nails it; the composer performs a whole bunch of sanity checks on the pipeline, quite aside from the produces/consumes. It also tries to give nice explanations of what was done wrong. We could maybe try to encode some of that using parametric roles, but I'm not sure we can get all the way, and I'm very sure the errors would make a whole lot less sense to user.

@vendethiel
Copy link
Member Author

It's interesting that this issue doesn't state a single reason why it would be better using a parametric role. :-)

An attempt to get something compile-time checkable, is, of course, desirable, which is not doable AFAIK with such methods. I didn't argue much for it because of the two latter points reducing a lot of the usefulness (while composeN can instead be rewritten as N-1 composes (I guess in some crazier languages like C++, you could actually type it correctly, using variadic generics.)).

Something like CompositeTransform also doesn't seem, from a very quick glance, to check that the steps inbetween the first and the last match their consume/produces. Apologies if that's actually the case (it's probably done anyway when the pipeline is running).

I probably feel like it could/should/would be nice to have types because such a pipeline/Supply in general feels very much like a map operation (except async/reactive and other differences).

I'm starting to realize something that's "missing" (probably by design) which would enable much more interesting transformations would be sub f(List[::T] --> ?[T]){}.

@vendethiel
Copy link
Member Author

Going to close this one, as without static checking for role it's not going to be of that much use.

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

No branches or pull requests

2 participants