Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameterized makeClassy #49

Closed
orenbenkiki opened this issue Sep 16, 2012 · 10 comments
Closed

Parameterized makeClassy #49

orenbenkiki opened this issue Sep 16, 2012 · 10 comments

Comments

@orenbenkiki
Copy link

Here is a possible approach for implementing makeClassy for parameterized containers. It works without any type-specific logic, as long as all the types are functional dependencies of the container type. This is something I sorely need in my code...

Sample code:

{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE FunctionalDependencies #-}
{-# LANGUAGE MultiParamTypeClasses #-}
{-# LANGUAGE ScopedTypeVariables #-}
{-# LANGUAGE TemplateHaskell #-}

module Sample where

import Control.Lens hiding (Simple)

-- Unparameterized classy containers are easy (yey!)

data Trivial = Trivial { _trivialField :: Int }

makeClassy ''Trivial

trivialAdd :: HasTrivial container => Int -> container -> Int
trivialAdd int container = int + container ^. trivialField

data TrivialContainer = TrivialContainer { _trivialContainer :: Trivial }

makeLenses ''TrivialContainer

instance HasTrivial TrivialContainer where
  trivial = trivialContainer

trivialExample :: Int -> TrivialContainer -> Int
trivialExample = trivialAdd

-- Parameterized containers seem possible:

data Simple num = Simple { _simpleField :: num }

makeLensesFor [ ("_simpleField", "__simpleField") ] ''Simple

-- Functional dependency to be compatible with the general case below.
class HasSimple container num | container -> num where
  simple :: SimpleLens container (Simple num)

instance HasSimple (Simple num) num where
  simple = id

simpleField :: forall container num . HasSimple container num => SimpleLens container num
simpleField = simple . __simpleField

simpleAdd :: forall container num . (HasSimple container num, Num num) => num -> container -> num
simpleAdd num container = num + container ^. simpleField

data SimpleContainer = SimpleContainer { _simpleContainer :: Simple Int }

makeLenses ''SimpleContainer

instance HasSimple SimpleContainer Int where
  simple = simpleContainer

simpleExample :: Int -> SimpleContainer -> Int
simpleExample = simpleAdd

-- This even works for multiple types which become "phantom" for some fields,
-- as long as all types are functional dependencies of the container:

data Complex foo bar = Complex { _complexFoo :: foo, _complexBar :: bar }

makeLensesFor [ ("_complexFoo", "__complexFoo"), ("_complexBar", "__complexBar") ] ''Complex

-- The functional dependencies here are required for this approach to work.
class HasComplex container foo bar | container -> foo, container -> bar where
  complex :: SimpleLens container (Complex foo bar)

instance HasComplex (Complex foo bar) foo bar where
  complex = id

complexFoo :: forall container foo bar . HasComplex container foo bar => SimpleLens container foo
complexFoo = complex . __complexFoo

complexBar :: forall container foo bar . HasComplex container foo bar => SimpleLens container bar
complexBar = complex . __complexBar

complexAddFoo :: forall container foo bar . (HasComplex container foo bar, Num foo) => foo -> container -> foo
complexAddFoo foo container =
  foo + container ^. complexFoo

complexAddBar :: forall container foo bar . (HasComplex container foo bar, Num bar) => bar -> container -> bar
complexAddBar bar container =
  bar + container ^. complexBar

data ComplexContainer = ComplexContainer { _complexContainer :: Complex Int Int }

makeLenses ''ComplexContainer

instance HasComplex ComplexContainer Int Int where
  complex = complexContainer

complexExample :: Int -> ComplexContainer -> Int
complexExample int container = complexAddFoo 1 container + complexAddBar 1 container
@ekmett
Copy link
Owner

ekmett commented Sep 16, 2012

One problem is that there are a number of cases this approach can't handle, such as if one of the arguments to ComplexContainer is a phantom type.

I do agree that it would be much nicer if makeClassy could just do the right thing for parameterized types and I'm willing to take another whack at improving it.

@ghost ghost assigned ekmett Sep 16, 2012
@orenbenkiki orenbenkiki reopened this Sep 18, 2012
@orenbenkiki
Copy link
Author

Sorry, open/close was due to a careless button click. At any rate, I have been using makeClassy in my code (expanding it manually for now when there are parameters) and it helped me remove an unsightly layer of boilerplate - I'm pretty happy with the concept. I wish there was a very terse way to say "record Y has only one field containing record X, so generate instance HasX Y ... for me" - but that's a small price to pay.

@orenbenkiki
Copy link
Author

I find makeClassy to be a wonderful aid for making code more compositional. It allows me to define small records with associated logic and then easily create a specific combination of these small pieces which can easily apply both the original independent pieces of logic and, when needed, additional logic gluing them together. It is the closest thing to "mix-ins" I know of on Haskell, with minimal boilerplate - way less than my previous approach. "Less code, more applaud".

The bliss is only hampered by (1) the lack of support for parameterized types, which is why I opened this issue; and (2) the need to specify the "instance HasFoo Bar where foo = barFoo" all over the place, which can be avoided using the following "passthrough" trick:

data Foo = Foo { _fooInt :: Int }

makeClassy ''Foo
-- Generates fooInt lens and also class HasFoo

data Bar = Bar { _barInt :: Int, _barFoo_ :: Foo }

makeClassy ''Bar
-- Generates barInt and barFoo lenses (note no trailing _), and of course class HasBar.
-- Since _barFoo_ ends with a _, also generates "instance HasBar a => HasFoo a where foo = barFoo

getFooIntOfBar bar = bar ^. fooInt

Mnemonics: in _field_ :: Type, the field is a "pass through" to the Type (which must be in the HasType class).

Implementation-wise, it seems like this would require adding a "_lensInstance" field to LensRules, allowing certain fields to trigger creating "classy" instances (this seems unrelated to the existing CreateInstance).

Does that sound like a reasonable addition to makeClassy?

@ekmett
Copy link
Owner

ekmett commented Sep 19, 2012

Lets put it in as a separate issue. I'm not entirely sure how I feel about it. It could be useful but its also very 'magic', and probably not the right default behavior. Right now makeClassy has a pretty fixed scope (and an exceeedingly hairy implementation that has enough intrinsic complexity that i'm loathe to extend its scope in more ad hoc directions).

That said, I don't have a particular objection to some kind of makeInstances ''Foo call that goes through and hunts for member fields that have types for which there is a HasBar class, or something and/or making a flag to turn this behavior on if it doesn't wind up having to thread through the rest of the code for makeClassy.

@ghost ghost assigned shachaf Nov 12, 2012
@ghost ghost assigned mgsloan and ekmett Nov 21, 2012
@ekmett
Copy link
Owner

ekmett commented Nov 25, 2012

Dusting this off and starting to work on it again.

ekmett added a commit that referenced this issue Nov 25, 2012
@ekmett
Copy link
Owner

ekmett commented Nov 25, 2012

I have this working. It passes the test suite, but could stand to have someone beat on it for a while.

@ekmett
Copy link
Owner

ekmett commented Nov 25, 2012

I'm going to close this out as ready to go.

It really at this point "works for me", but could use a lot more testing.

@ekmett ekmett closed this as completed Nov 25, 2012
@lingnand
Copy link

I'm wondering if we should move the container to the second parameter in the class declaration? By convention the later parameters are more important and this will also enable people to do newtype deriving.

@ekmett
Copy link
Owner

ekmett commented Jan 13, 2016

I'd recommend opening that as a separate issue, I suspect we'll likely close it as wontfix / rejected given the sheer amount of code that would be broken, but it'd be better served as an issue than as a comment on a 3 year old issue nobody is looking at any more.

@lingnand
Copy link

Cool, thanks for the suggestion - I've moved it to #625

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants