search

byteverse / disjoint-containers

Other
6 stars 4 forks source link

Get rid of transformers dependency #2

Open andrewthad opened 7 years ago

andrewthad commented 7 years ago

Transformers is used in a single place in the code. The StateT/MaybeT monadic binds could just be inlined. The result would be cleaner and may possibly perform better.

chessai commented 5 years ago

Not sure this is too big of a deal since we must transitively depend on transformers via primitive.

One could argue that it's a maintenance overhead, but transformers introduces breaking changes so infrequently that this doesnt seem like a big deal.

chessai commented 5 years ago

Oh, I had the wrong library in my head when writing this. We don't depend on primitive.

andrewthad commented 5 years ago

I don’t see much value in getting rid of the Transformers dependency since I’ve never seen an application that doesn’t somehow transitively depend on transformers, but if someone wanted to get rid of the dependency, I would take a PR. The only place we use transformers is one function that helps perform union. This function uses a transformer stack consisting of maybe and state. It would be possible instead to either define a single data type that flattens the stack along with various helper functions. Or it would be possible to not define a data type at all and just in line the control flow and state modifications.

chessai commented 5 years ago

Would there actually be any value in such a PR?

andrewthad commented 5 years ago

Not unless there's someone out there building an application that depends on disjoint-containers but not transformers.

chessai commented 5 years ago

Can we just close this then? I don't think that's probable

chessai commented 5 years ago

oops, didn't mean to close just yet