search

ivan-m / graphviz

Haskell bindings to the Graphviz toolkit
Other
64 stars 24 forks source link

class NodeList n nl breaks type inference with OverloadedStrings #17

Closed lspitzner closed 7 years ago

lspitzner commented 7 years ago

I.e. the example in "Data.GraphViz.Types.Monadic" does not compile with or without OverloadedStrings anymore, either due to ambiguities or too-specializisities.

I can see useful applications of typeclasses with the purpose of overloading functions, but this instance (the NodeList, .. class, pun not initially intended) i'd certainly see as overuse of the pattern.

lspitzner commented 7 years ago

oh, and thanks for providing this library!

ivan-m commented 7 years ago

Looks like my solution for #4 is what's breaking everything here...

:man_facepalming:

I'm going to have a play and try and work out how to deal with this, but it might mean rolling back that solution (which IIRC was why I never did it that way in the first place).

EDIT: right, you said right up front what was causing the issue, whereas I just went straight to "why doesn't the sample work".

Possibly the typeclass is an overuse; I just went "hey, this works and type-checks! cool!".

(I have had people tell me I like ad-hoc typeclasses a bit too much...)

ivan-m commented 7 years ago

Specifically, the problem is the clash of type-classes: NodeList and ones like IsString, Num (for numeric literals), etc.

Since the likely primary usage of this particular module is for static, literal graphs, I think I'll roll back the NodeList solution and go with one based upon what @haroldcarr originally proposed.

lspitzner commented 7 years ago

i'd even go so far and say only --> :: n -> n -> DotM n () is sufficient, because there is for_:

["a1", "a2", "a3"] `for_` \a -> a --> "b"
-- or even
["a1", "a2", "a3"] `for_` (--> "b")

But I am not opposed to special operators either, if others prefer them.

ivan-m commented 7 years ago

I never even considered traverse_, for_, mapM_, forM_, etc. ... (even though I use mapM_ under the hood!).

In that case, I'm going to roll back 0f9c228 and replace it with documentation saying to do it manually.

lspitzner commented 7 years ago

thanks, works well. Fyi, I have used this library to describe dataflow of my haskell source formatter, see the result and its source.