docmunch / antemodulum

Yet another prelude
MIT License
0 stars 0 forks source link

avoid ad-hoc typeclasses #3

Open gregwebs opened 10 years ago

gregwebs commented 10 years ago

Antemodulum.Text introduces ad-hoc type-classes. This is essentially what classy-prelude used to do and decided was a bad idea because the type inference was bad, error messages were poor, and it was not that easy to reason about the code. In part this was due to generality. Antemodulum.Text will probably work fine because it only works on Text. But we would like generality without much downside by building on foundational type classes.

In the case of Text there is an EqSequence and Textual typeclass that should be able to handle our needs. http://hackage.haskell.org/package/mono-traversable-0.2.0.0/docs/Data-Sequences.html#t:EqSequence

With respect to FilePath that seems ok since we don't have another option. But instance IsFilePath Text is not a generally safe conversion so I am not sure how I feel about that, particularly if this is open source for more than just docmunch.

spl commented 10 years ago

I agree that there is a limit to what one should do with a type class, but I'm not sure what you mean by “ad-hoc” and “foundational” and the difference between the two. (And I don't think you mean “ad-hoc” in the sense of “ad-hoc polymorphism,” which is a term generally used for the type of generalization provided by type classes.)

Regarding Antemodulum.Text, the Strippable class will probably not cause any problems with type inference, except in the sense of the monomorphism restriction. I'm not sure if you meant that EqSequence and/or Textual could replace Strippable, because I don't see how.

Regarding IsFilePath, I don't have a strong preference about it. I just see that it is basically the only way ever used to convert a FilePath to Text.

gregwebs commented 10 years ago

Can you get rid of Antemodulum.Text and use EqSequence or Textual?

gregwebs commented 10 years ago

I will look at adding stripping to the Textual typeclass.

spl commented 10 years ago

Just to add my point from our conversation: the whitespace-stripping functions are defined as follows in both strict and lazy Text modules:

strip = dropAround isSpace
stripStart = dropWhile isSpace
stripEnd = dropWhileEnd isSpace

And then there is:

dropAround p = dropWhile p . dropWhileEnd p

Since dropWhile is already in IsSequence, it seems like the smallest and most useful change would be to add dropWhileEnd to IsSequence. Then, the remaining functions could be defined with IsSequence and isSpace.