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

Can we use type classes to eliminate need for attr and attr' functions? #11

Open
tgdwyer opened this issue Apr 10, 2015 · 6 comments
Open

Comments

@tgdwyer
Copy link
Contributor

tgdwyer commented Apr 10, 2015

Many thanks for creating purescript-d3, I'm having fun learning purescript by trying to convert js d3 demos.

I'm a purescript newbie just learning about type classes so sorry if I'm naive, but I wonder if it would be possible to create a typeclass for the possible arguments to the Selection attr functions, such that we can have a single overloaded attr that takes a string, a number or a function? I miss that aspect of the original javascript interface.

Seems simple enough to make a toy example - what am I missing?

class Helloable a where
  attr :: a -> String
instance helloNumber :: Helloable Number where
  attr n = "hello " ++ (show n)
instance helloString :: Helloable String where
  attr s = "hello " ++ s
instance helloFunc :: Helloable (String->String) where
  attr f = f " hello"

test = attr "there" ++ (attr 4) ++ (attr (\s -> "the function says " ++ s))

Thanks again!

@tgdwyer
Copy link
Contributor Author

tgdwyer commented Apr 11, 2015

In fact, looking at the code for Selection.purs I see that there's the attr function is already overloaded for String and Number:

class AttrValue a
instance attrValNumber :: AttrValue Number
instance attrValString :: AttrValue String

Adding a third instance allows me to pass functions into the regular attr:

instance attrValFunc :: AttrValue (d->v)

But I guess it would be better to have some restrictions on the return value of the function...

@pelotom
Copy link
Owner

pelotom commented Apr 12, 2015

Hey, glad you like the library and thanks for your contributions!

This seems like a great idea, and I've played around with trying to do something like it, but I've run into problems that I don't know how to solve. As you mentioned, the tricky part is that you want to constrain the types involved... actually not just the return value v, but also the data type d, because it should match the currently bound data of the selection. So let's say we want to introduce a new method attr2:

class Existing s where
  -- ...
  attr2 :: forall d v. (AttrValue2 v d) => String -> v -> s d -> D3Eff (s d)
  -- ...

with the idea that it will be able to supersede attr and attr' (and ultimately attr'', but let's leave that aside for now). It has a constraint AttrValue2, which is parameterized not just by the value type v but also the data type d:

class AttrValue2 v d

So to have it do the duty of attr we just need to make any regular AttrValue also an AttrValue2 instance:

instance liftAttrVal :: (AttrValue v) => AttrValue2 v d

That seems to work fine; you can replace all instances of attr with attr2 in the examples and they still compile. The next step is to replace attr', so to do that we might add an instance:

instance attrValFun1 :: (AttrValue v) => AttrValue2 (d -> v) d

Unfortunately this doesn't seem to work as expected. purescript-d3 will compile by itself if you add this instance, but if you try to use it in the examples, replacing call sites of attr' with attr2, you'll run into trouble. Essentially it looks like the d parameter is not unifying with the data type, so you can pass functions to it from any type to a simple attribute value type. This seems like a compiler bug to me... I've noticed a bit of wonkiness in general with multiparameter typeclasses in PureScript. Anyway, that's the only obstacle I see with this approach. If this worked, then we could go on to define

instance attrValFun2 :: (AttrValue v) => AttrValue2 (d -> Number -> v) d

And thereby supersede attr'' as well. Then we could go and do the same for all the other foo'/foo'' methods in Graphics.D3.Selection! It would be a very nice simplification, for sure.

@tgdwyer
Copy link
Contributor Author

tgdwyer commented Apr 12, 2015

Thanks for the response! Is it worth logging an issue about multi-parameter typeclasses over at purescript?

@pelotom
Copy link
Owner

pelotom commented Apr 12, 2015

Yeah, definitely worth filing a bug on it!

@pelotom
Copy link
Owner

pelotom commented Apr 12, 2015

If you make an issue, @-mention me so I can watch it and participate in the discussion.

@tgdwyer
Copy link
Contributor Author

tgdwyer commented Apr 18, 2015

Actually, I'm going to pass on filing the bug on the purescript compiler myself. I had a quick go at making a minimal repro of the problem... but quickly realised when things started going wrong I couldn't tell whether it was my fault or the compiler. While it would be a great exercise for me to get to the bottom of it, I just don't have time at the moment. Sorry!

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

No branches or pull requests

2 participants