search

ndmitchell / extra

Extra Haskell functions
Other
94 stars 37 forks source link

Add compareLength and comparingLength #71

Closed josephcsible closed 3 years ago

josephcsible commented 3 years ago

Comparing the output of length is an anti-pattern, since it requires unnecessarily walking the entire Foldable. Add two replacements for it.

josephcsible commented 3 years ago

The AppVeyor failure appears unrelated to these changes.

josephcsible commented 3 years ago

Would be good if the tests were instead written as doc comments, and generated, since thats how most tests are written, its less maintenance, and its useful documentation for users.

This was my preference too, but that doesn't actually work because of this: https://github.com/ndmitchell/extra/blob/fcda15f94158fbde1a9a2864a987d352be6f7168/Generate.hs#L73

ndmitchell commented 3 years ago

Hmm, I guess that means we should probably have them in Data.List.Extra with the more list-based types, and have the tests on that - then we can implement some (e.g. comparingLength) in terms of the list ones and get the tests that way.

josephcsible commented 3 years ago

Done. (At first I thought it might be weird to have things that work on Foldables in Data.List.Extra, but then I realized Data.List itself has a bunch of them too.)

ndmitchell commented 3 years ago

Thanks!

mixphix commented 3 years ago

Would be good if the tests were instead written as doc comments, and generated, since thats how most tests are written, its less maintenance, and its useful documentation for users.

This was my preference too, but that doesn't actually work because of this:

https://github.com/ndmitchell/extra/blob/fcda15f94158fbde1a9a2864a987d352be6f7168/Generate.hs#L73

How difficult would it be to make Data.Foldable.Extra doctest-able, by qualifying the module import? Pardon my ignorance, I'm new to library contributions 😬

ndmitchell commented 3 years ago

Wouldn't be too hard - main issue is in the doc snippets do you write F.foldable_function (looks a bit ugly? but maybe it's no big deal?) or foldable_function and then potentially try and auto-qualify them when generating the doctest snippets (probably not too hard because we've already computed the exports and know which names need qualifying? or we could let foldable_function = F.foldable_function in ... and go even simpler?). I've no idea which route to take. But given the very small number of foldable functions as yet, it doesn't seem to be too pressing (although I'd certainly welcome a PR fixing it).