basvandijk / case-insensitive

Case insensitive string comparison
Other
25 stars 19 forks source link

Derive Data class #10

Closed nikita-volkov closed 10 years ago

nikita-volkov commented 11 years ago

This solves #9

basvandijk commented 11 years ago

Deriving Data makes a datatype non-abstract (i.e. you can construct a value of the type without going through the safe interface, in this case Data.CaseInsensitive.mk).

I would rather keep CI abstract so that users will never rely on its implementation which means I can change it later on.

nikita-volkov commented 11 years ago

@basvandijk Does keeping it abstract really matter when all imaginable types that it makes sense to use with CI do have the Data instance? On the contrary side the absence of the Data instance makes it impossible to implement other types requiring Data in terms of CI. It is also impossible to manually declare deriving instance Data a => Data (CI a), because the constructors are not exported.

There's a bunch of libraries requiring types to have Data instance, and the aforementioned limitations render CI simply impossible to use with them.

Want to see a practical example? The acid-state library requires types to derive Data, and I simply can't declare the following type:

newtype Tag = Tag (CI Text)
  deriving (Eq, Ord, Show, Typeable, Data)
bos commented 10 years ago

I just hit this too.

basvandijk commented 10 years ago

Ok. I reconsidered it and concluded that while providing a Data instance is unsafe, users have to go out of their way to create an invalid CI.

I derived a Data instance and released it as 1.2.0.0.