search

alt-romes / hegg

Fast equality saturation in Haskell
https://hackage.haskell.org/package/hegg
BSD 3-Clause "New" or "Revised" License
75 stars 8 forks source link

Is the View Pattern really necessary here? #19

Closed folivetti closed 2 years ago

folivetti commented 2 years ago

I'm not very proficient with ViewPatterns, but in this particular line couldn't we just use:

auxs <- toList <$> traverse aux p

in my opinion it would make it much clearer what this line is doing.

https://github.com/alt-romes/hegg/blob/ef0837b7c7c55b826e496e29aeed454dbf9601a8/src/Data/Equality/Matching.hs#L118

alt-romes commented 2 years ago

@folivetti Surely, there's no particular reason for it. Might have sprung up from things moving around and ended up just being toList which I agree is not great.

Feel free to open a PR 👍 !

folivetti commented 2 years ago

Thanks! Will do that very soon, I'm taking my spare time to read and understand all your code, hopefully I'll start contributing by next month, including the Debug implementation.

alt-romes commented 2 years ago

Thanks! Will do that very soon, I'm taking my spare time to read and understand all your code, hopefully I'll start contributing by next month, including the Debug implementation.

It's very exciting to hear so!

I have cooking the redesign of Analysis in another branch, which will change up some things, but I haven't found time to finish it yet

Thanks!