A scala anti pattern -- looking for a name
Henry Story
A committer on the Apache Clerezza incubator project has discovered a new anti-pattern.
He has written some code that requires one to weave import statements into one's code if it is going
to give the right results. Here is an example:
val ez1: EZMGraph...
val ez2: EZMGraph...
import ez1._
ez1.b_("reto") -- FOAF.homepage --> "http://farewellutopic.com/".uri
import ez2._
ez2.b_("hjs") -- FOAF.homepage --> "http://bblfish.net/".uri
import ez1._
ez1.b_("reto") -- FOAF.currentProject --> "http://clerezza.org/".uri
If the second or third imports are not added, the wrong graph container will be
filled with the relation.
I believe that any code that requires one to do that is evidently an anti-pattern.
But you may be intrigued to find out how one can generate such an anti-pattern in Scala?
The answer in short is by writing an implicit with an hidden variable
protected trait TcDependentConversions extends TcIndependentConversions {
def baseTc: TripleCollection
implicit def toRichGraphNode(resource: Resource) = {
new RichGraphNode(new GraphNode(resource, baseTc))
}
}
and then writing a class that extends that trait
class EzMGraph(val baseTc: MGraph) extends AbstractMGraph with TcDependentConversions
In fact the classes are all here:
I wrote a patch for this code https://issues.apache.org/jira/browse/CLEREZZA-603
but I am told that this cannot be accepted until a name for this anti-pattern has
been found. I therefore suggest that we call it the Reto anti-pattern after the name
of its author and in honor of him. But perhaps he was not the first to discover it,
and we should not be assigning an original claim where it is not warranted.
Henry
Social Web Architect
http://bblfish.net/
Josh Suereth
Chris Lewis
It seems one of the most overused and abused features to scala
new-comers is implicit conversions. They're useful sometimes, but a real
pain lots of times. I'll defer to Josh Suereth's talks on why, but that
code snippet is a shining example.
-chris
Henry Story
I'd call this 'too much pimpin'.
THere's probably a simpler no-implicit DSL that you could provide that looks and feels the same, but without the pitfalls.
Josh Suereth
On 19 Jul 2011, at 16:31, Josh Suereth wrote:I'd call this 'too much pimpin'.Good name. But certainly the "Pimpin Reto" anti-pattern, would do it's author a lot more credit though?THere's probably a simpler no-implicit DSL that you could provide that looks and feels the same, but without the pitfalls.Yes, that is what the patchdoes. It removes the inheritance from the trait containing the implicit, and creates a few explicitmethods that have the same effect. In my view the code is a lot more readable too as a result.Henry
Henry Story
On 19 Jul 2011, at 16:40, Chris Lewis wrote:
> "Implicitly Aroused."
>
> It seems one of the most overused and abused features to scala new-comers is implicit conversions. They're useful sometimes, but a real pain lots of times. I'll defer to Josh Suereth's talks on why, but that code snippet is a shining example.
Yes, I completely agree. In fact that is what another of my patches was out to solve, the one I
posted in order to solve "EzMGraph imports too many implicits"
https://issues.apache.org/jira/browse/CLEREZZA-599
There the author imported a dozen or more implicits into a class. As a result when one uses an IDE one gets a bunch of methods that appear that have nothing to do with the class. I posted some pictures that show this anti-pattern in action on the CLEREZZA-599 issue.
Chris Lewis may want to use this as an example in his future rants.
My patch was not accepted, probably again because the author of the code wishes his name to be
associated with this anti-pattern first.
Henry
Nils Kilden-Pedersen
Good name. But certainly the "Pimpin Reto" anti-pattern, would do it's author a lot more credit though?
Carl-Eric Menzel
Chris Lewis <ch...@thegodcode.net> wrote:
> "Implicitly Aroused."
>
> It seems one of the most overused and abused features to scala
> new-comers is implicit conversions. They're useful sometimes, but a
> real pain lots of times. I'll defer to Josh Suereth's talks on why,
> but that code snippet is a shining example.
I get the impression that implicits are suffering from @Syndrome.
Remember when annotations were all new and shiny with Java 1.5 and
people sprinkled them everywhere like magic pixie dust? Looks like
implicits are just as shiny and fun right now as annotations used to be.
Carl-Eric
Henry Story
√iktor Ҡlang
Viktor Klang
Akka Tech Lead
marc
Henry Story
Winner.
On Tuesday, July 19, 2011 8:46:24 AM UTC-7, √iktor Klang wrote:"Gimp my Library"
2. | slang a sexual fetishist who likes to be dominated and whodresses in a leather or rubber body suit with mask, zips, and chains |
On Tue, Jul 19, 2011 at 5:24 PM, Josh Suereth <joshua....@gmail.com> wrote:Better to make a viral meme. "Oh, don't do that, or implicit-cat will get you"On Tue, Jul 19, 2011 at 10:53 AM, Nils Kilden-Pedersen <nil...@gmail.com> wrote:On Tue, Jul 19, 2011 at 4:43 PM, Henry Story <henry...@gmail.com> wrote:Good name. But certainly the "Pimpin Reto" anti-pattern, would do it's author a lot more credit though?I'm not sure I'd want an anti-pattern named after me. Does the author?
--
Viktor Klang
Akka Tech Lead
√iktor Ҡlang
On 19 Jul 2011, at 18:08, marc wrote:Winner.
On Tuesday, July 19, 2011 8:46:24 AM UTC-7, √iktor Klang wrote:"Gimp my Library"is this "gimp" in the sense of
2.slang a sexual fetishist who likes to be dominated and whodresses in a leather or rubber body suit with mask, zips, and chains
You can pick your own definition ;-)
On Tue, Jul 19, 2011 at 5:24 PM, Josh Suereth <joshua....@gmail.com> wrote:Better to make a viral meme. "Oh, don't do that, or implicit-cat will get you"On Tue, Jul 19, 2011 at 10:53 AM, Nils Kilden-Pedersen <nil...@gmail.com> wrote:On Tue, Jul 19, 2011 at 4:43 PM, Henry Story <henry...@gmail.com> wrote:Good name. But certainly the "Pimpin Reto" anti-pattern, would do it's author a lot more credit though?I'm not sure I'd want an anti-pattern named after me. Does the author?
--
Viktor Klang
Akka Tech Lead
Reto Bachmann-Gmür
Importing the members of an instance makes only sense when that instance has some instance fields (otherwise you could use an object). I think the import-statement can be quite nicely used to set a context. (The example of Henry is confusing as it calls b_ on a graph-instance, the EzGraph code is however designed to easily write down rdf graphs in scala, similarly to the turtle language it is designed for a single graph).
An example on how I think imports can be used to set the context:
val companyA : EzCompany = ..
val companyB : EzCompany = ...
val henry: Person = ..
import companyA._
henry.paySalary(); // The person is implicitly converted to a an Employe of companyA,
henry.fire();
henry.payServerancePackage()
import companyB._
henry.hire() // The person is implicitly converted to a an Employe of companyB
henry.paySalary(); //this calls transfers money from companyB to henry
The EzMGraph DSL to create RDF graphs in clerezza is similar
val uriA: UriRef
val uriB: UriRef
val uriC: UriRef
val uriD: UriRef
//here we add triples to ezGraphB
import ezGraphA._
//FOAF.knows and name are UriRefs, in RDF UriRefs are both property-types as well as nodes
uriA -- FOAF.knows --> uriB
uriB -- FOAF.name -->"Alice"
//here we add triples to ezGraphB
import ezGraphB._
uriA -- FOAF.knows --> uriC
uriC -- FOAF.knows --> uriD
assuming the two graphs were empty at the beginning they now have exactly two triples.
Reto
Josh Suereth
Rex Kerr
In your example, henry contains a bunch of mutable state. You could at _least_ have the company explicitly named:
henry.asEmployeeOf(companyA){ henry =>
henry.paySalary()
if (!henry.reviewCode) henry.fire()
}
Saving a little typing while losing critical context cues is almost always the wrong thing to do.
--Rex
HamsterofDeath
Chris Twiner
understand if you were upset by it....
I do have to say this code scares me as its shown, seeing:
// join new company
val happyHenry : Employee = henry.joinCompany(companyA, startDate)
// leave old
henry.resign().hopeForServerancePackage()
val happierHenry : PaidEmployee = companyA.pay(happyHenry)
val upsetHenry : FiredEmployee = companyA.fire(happyHenry)
....
instead of
import companyA._
henry.paySalary(); // The person is implicitly converted to a an
Employe of companyA,
henry.fire();
henry.payServerancePackage()
is clearer and less risky even if it is more wordy.
The problem with that code is that if you // import companyA._, you
get a big oops occur. Or you have a higher precedence import
elsewhere --> oops.
I don't think this can be called an anti-pattern regardless, its got
to have been done more than a few times by different people to be any
kind of pattern.
Its not code I'd want to use but that doesn't seem to justify this
kind of public spectacle.
That said, I think you are taking this very well and should be
respected for that.
<snip/>
Jim Balter
Reto wrote "I think it's important to identify anti-pattern. Post a
link here if you document the anti-pattern you think you've discovered
on the scala mailing list" and then, here, "I'm eager to understand
what this anti-pattern is about". He asked for a public *airing*, and
he's getting it. I don't think there's any "offense" or "spectacle" --
that might be the case if posters really were naming the anti-pattern
after him, but they aren't -- other than Henry, who obviously is
unhappy about his patch being rejected. Everyone should be paying
attention to http://en.wikipedia.org/wiki/Egoless_programming . And
there really is an anti-pattern here that has been identified --
excessive and improper use of implicits that discard type safety.
-- Jim
Maxime Lévesque
I'd argue that there is a more generic anti pattern : Discarding Type Safety,
there are plenty to lose type safety. This being is a specific case of it,
does it deserve it's own anti pattern ?
Jim Balter
identified this as a specific problem, particularly among people new
to the language. And I really don't think "discarding type safety"
qualifies as a (anti-) pattern.
-- Jim
2011/7/19 Maxime Lévesque <maxime....@gmail.com>:
Rex Kerr
Andthere really is an anti-pattern here that has been identified --
excessive and improper use of implicits that discard type safety.
Although the code is not type-safe, there are lots of other patterns that also are not type-safe and do not have this particular flaw.
The flaw here is _non-obvious context_. It's not so much a problem with types as it is with _labels_.
Of course, there is a limit where one has no variable names, only types; all objects of the same type are the same. But since that's not how most languages work, it's important to draw the distinction.
Look, we can rewrite the same pattern without sneaky imports of implicits:
companyAize
henry.doWhatever
companyBize
henry.doWhateverElse
where companyAize sets a mutable variable that's read by henry.doWhatever (likewise with B and doWhateverElse).
This is bad coding practice because it is hard to follow the state changes, especially as code gets more complex or needs to be added.
Doesn't matter whether it's a hidden global variable, a hidden mutable class member, or a hidden implicit parameter. The problem is hidden yet varying context-sensitivity in each case.
--Rex
Derek Chen-Becker
ignorantly) removes an import statement then all hell breaks loose.
Implicits can be great when used *judiciously* to extend functionality
or "lubricate" the process of converting between two types (even then,
be careful), but this just appears to be trying to avoid a little extra
typing by making the context of the code itself implicit. There's plenty
of room to argue exactly where the right balance of conciseness and
readability lies, but to me the sample code you've shown here is just
setting its maintainers up for some nasty surprises.
Derek
Jim Balter
Yes, this looks like the correct identification. There may be a whole
set of problems that can result from promiscuous use of implicits, but
that too is not a pattern per se, whereas this is, and isn't limited
to Scala or implicits.
-- Jim
bblfish
val ez1: EZMGraph...
val ez2: EZMGraph...
import ez1._
ez1.b_("reto") -- FOAF.homepage --> "http://farewellutopic.com/".uri
import ez2._
ez2.b_("hjs") -- FOAF.homepage --> "http://bblfish.net/".uri
import ez1._
ez1.b_("reto") -- FOAF.currentProject --> "http://clerezza.org/".uri
You can write
val ez1: EZMGraph...
val ez2: EZMGraph...
ez1.b_("reto") -- FOAF.homepage --> "http://farewellutopic.com/".uri
ez2.b_("hjs") -- FOAF.homepage --> "http://bblfish.net/".uri
ez1.b_("reto") -- FOAF.currentProject --> "http://clerezza.org/".uri
And it does the right thing.
Perhaps I did not emphasize that enough in the bug report or the patch submission.
Reto Bachmann-Gmür
EzGraphNode(uriA, graphA) -- FOAF.knows --> uriB
EzGraphNode(uriB, graphA) -- FOAF.name -->"Alice"
EzGraphNode(uriA, graphB) -- FOAF.knows --> uriC
EzGraphNode(uriC, graphB) -- FOAF.knows --> uriD
The general idea I tried to show with the person2employee example is the dynamic applying of context specific decorators. Many different systems deal with Persons in different ways, decorators may offer this context specific methods.
E.g.:
Explicit variant:
val henryCath = new PersonAsSeenByRomanCatholicChurch(henry)
henryCatch.beatify() //thows AccessControlException if PopePermission missing
Implicit Variant:
import RomanCatholic._
henry.beatify()
If another decorator has a method with the same name, we might run into the situation which seems to be perceived as the problem justifying naming this an anti-pattern:
import RomanCatholic._
henry.baptise()
import Wicca._
henry.beatify()
import RomanCatholic._
henry.exCommunicate
Obviously without the "import Wicca._" statement something rather weird happens above. Personally I think this problem should be tacked by limiting the scopes of the imports, e.g. by adding braces around the 3 import scopes in the example rather than deprecating the pattern (calling it an anti-pattern).
Now to come back to the issue Henry original raised, the question is if methods of say PersonAsSeenByRomanCatholicChurch that return a person should rather return PersonAsSeenByRomanCatholicChurch or just Person. While returning a PersonAsSeenByRomanCatholicChurch may obsolete another implicit conversions I think its generally better to apply the decorator only when and if needed.
An example to illustrate desired changing of scope:
val heretics: Set[Person] = ...
heretics += henry
//lets add the whole wicca-enotourage of the heretic
import Wicca._
heretics += henry.bloodBrother
heretics += henry.godfather
heretics += henry.priest
import RomanCatholic._
for (h <- heretics) {
h.excommunicate()
}
Above clearly we want to excommunicate from the roman catcholic church and not from Wicca, this works correctly if the Wicca-decorator methods bloodBrother, godfather and priest returned a Person and not a WiccaPerson, as otherwise WiccaPerson.excommunicate would be invoked.
Reto
Philippe Lhoste
> ( http://dictionary.reference.com/browse/gimp )
Wow, I didn't know this word has so many senses!
I only knew the GNU Image Manipulation Program, which was probably the original sense of
this klangism (or at least the way I interpreted it).
--
Philippe Lhoste
-- (near) Paris -- France
-- http://Phi.Lho.free.fr
-- -- -- -- -- -- -- -- -- -- -- -- -- --
Reto Bachmann-Gmür
Creating RDF-Graphs as anonymous subclasses
val ez1: MGraph = new EZMGraph() {(
bnode -- FOAF.homepage --> "http://farewellutopic.com/".uri
Which is a shortcut for
val ez1: MGraph = new SimpleMGraph() {(
val bnode = new BNode()
add(new TripmeImpl(bnode, FOAF.homepage, new UriRef("http://farewellutopic.com/"))
add(new TripmeImpl(bnode, FOAF.curentProject, new UriRef("http://clerezza.org/"))
)}
While the example above wouldn't necessarily need implicit conversion as the bnode method of EzMGraph could return something like a RichBNode I see no good alternative to implicit conversions in case like in the example in my fisrt mail in this thread, or for example to allow the following code
// a graph with a single triple:
// foaf:Person rdf:type rdfs:Class
val simple: MGraph = new EZMGraph() {
FOAF.Person a RDFS.Class
}
Reto
Henry Story
In some cases the benefits of having a simple API might outweigh the disadvantage of the possibility of a runtime exception. I agree that the readability gain is questionable in general, in the concrete RDF-Graph example in my mail I think the gain is quite big as it would otherwise be
EzGraphNode(uriA, graphA) -- FOAF.knows --> uriB
EzGraphNode(uriB, graphA) -- FOAF.name -->"Alice"
EzGraphNode(uriA, graphB) -- FOAF.knows --> uriC
EzGraphNode(uriC, graphB) -- FOAF.knows --> uriD
graphA2.node(uriA) -- FOAF.knows --> ( uriB -- FOAF.name --> "Dan Brickley" )graphB2.node(uriA) -- FOAF.knows --> ( uriC -- FOAF.knows --> uriD )
val graphA3 = new EzMGraph() {node(uriA) -- FOAF.knows --> ( uriB -- FOAF.name --> "Dan Brickley" )}val graphB3 = new EzMGraph() {node(uriA) -- FOAF.knows --> ( uriC -- FOAF.knows --> uriD )}
The general idea I tried to show with the person2employee example is the dynamic applying of context specific decorators. Many different systems deal with Persons in different ways, decorators may offer this context specific methods.
Reto Bachmann-Gmür
I see an example of a very poor usage of a library but I cannot identify the documentation of an anti-pattern. It would help to see in a generic way the kind of problem one wants to adress, how the anti-pattern seems to address the issue, why at the end it causes more harm than benefits and last but not least the documented refactored solution that solves the problem without causing the harm.
Then we can see if the anti-pattern is indeed implemented in the clerezza library or in henry's strawman code.
> and then, here, "I'm eager to understand
> what this anti-pattern is about". He asked for a public
*airing*, and
> he's getting it. I don't think there's any "offense" or
"spectacle" --
> that might be the case if posters really were naming the
anti-pattern
> after him, but they aren't -- other than Henry, who obviously
is
> unhappy about his patch being rejected.
Not strictly true, I asked for an explicit vote and critized parts of the patch that are not necessary to fix the described problem (which I see rather as a constructed problem rather than a real one, as it doesn't occur when using the dsl as intended and documented).
> Everyone should be paying
> attention to http://en.wikipedia.org/wiki/Egoless_programming
. And
> there really is an anti-pattern here that has been identified
--
> excessive and improper use of implicits that discard type
safety.
Imho implicit are a nice feature of scala bringing some of the advantages of dynamically typed languages to a static type-system. I don't see where type safety is discarded.
Reto
>
> -- Jim
Matthew Pocock
> there really is an anti-pattern here that has been identified --
> excessive and improper use of implicits that discard type safety.Imho implicit are a nice feature of scala bringing some of the advantages of dynamically typed languages to a static type-system. I don't see where type safety is discarded.
Reto
>
> -- Jim
--
Josh Suereth
Reto Bachmann-Gmür
> there really is an anti-pattern here that has been identified --
> excessive and improper use of implicits that discard type safety.Imho implicit are a nice feature of scala bringing some of the advantages of dynamically typed languages to a static type-system. I don't see where type safety is discarded.
I agree. It's not that this code circumvents type-safety, but that the repeated importing of things
a) silently shadows other things in scopeb) injects hidden mutable state
It is the combination of these two things that makes this strawman example of repeatedly importing instances such a wonderful example of something we should not do. It may well be reasonable in throw-away scripts, but not in anything you intend to maintain.
Regarding the shadowing, are there any compiler warnings that tell us when we've imported things that cause things in scope to be shadowed?
Of course shadowing imports is a potential source of bugs and difficulty to read the code. But isn't this true for many if not all features of a language? Scala introduces a cascading imports mechanism. It might be a reasonable design choice and increase readability and maintainability to only have import statements at the beginning of blocks, the strawman could easily be modified to follow this design.
Functional programming avoids mutable states, but I don't see what's particularly bad with "hidden" ones. Modification of a hidden mutable state is like a side effect on the environment. If we invoke
println("hi")
we rely on the mutable field of System with a PrintStream proxied by System.out.
Similarly we act on an evironment in kojo if we draw a shape by sending around and changing the state of a turtle with
forward(100)
right()
penUp()
forward(100)
right()
penDown()
forward(100)
On the clerezza console by default we act on the main content graph, entering the statement
zz>for (person <- FOAF.Person/-RDF.`type`) { "http://jibbering.com/foaf/santa.rdf".uri -- FOAF.knows --> person }
Would make sure that Santa knows everybody, where "everybody" is scoped to the world described by the content graph and this world is being changed. (Note that imho the smelliest part in the above is the uri method on string, this is possible thanks to a implicit conversion on String Henry introduced. It smells, but I like it)
With Henry's proposal the above would look like this
zz>for (person <- contentGraph.node(FOAF.Person)/-RDF.`type`) { contentGraph.node("http://jibbering.com/foaf/santa.rdf".uri) -- FOAF.knows --> person }
This would not reduce the implict conversions as contentGraph needs to be decorated with the node method but would unhide the mutable graph and avoid having a state that could be shadowed by an import. Similarly one could avoid the hidden mutable state in kojo
turtle.forward(100).right().penUp().forward(100).right().penDown().forward(100)
We could furthermore reduce the methods with side effects and have an have no mutable object at all. When the api is about working on mutable triplestores (graphs) this might be harder to achieve and lead to a less intuitive api.
I personally prefer both the current kojo as well as the current clerezza approach.
I think that dynamically scoping uris and blank nodes to the context of a graph not only saves typing but can also make the code more readable. I acknowledge that one could set priority differently and focus on other usecases BUT I cannot see the anti pattern Henry is looking a name for. A code smell maybe, which unlike an anti pattern doesn't give us a generic refactoring recipe but reminds us to evaluate the code carefully. In the concrete case my opinion is that the easiness of the DSL syntax outweigh the risk of having the shadowable context (that's for the library, I agree Henry's strawman would benefit from adding blocks avoiding the shadowing).
Reto
PS: According to wkipedia many anti-pattern ideas amount to little more than rants. Rants in turn can be seen as an anti-pattern for effective discussion and building of productive open source communities. Or is this again a rant?
Derek Chen-Becker
> Functional programming avoids mutable states, but I don't see what's
> particularly bad with "hidden" ones. Modification of a hidden mutable
> state is like a side effect on the environment. If we invoke
>
> println("hi")
>
> we rely on the mutable field of System with a PrintStream proxied by
> System.out.
Ah, but you change that mutable state (which stream is being written to)
via an explicit method call, not by an import. Using an import to modify
state is what I believe to be the anti-pattern here. Having personally
worked on a significant Scala codebase, I have to say that I would
strongly prefer Henry's approach that you outlined:
> for (person <- contentGraph.node(FOAF.Person)/-RDF.`type`) {
contentGraph.node("http://jibbering.com/foaf/santa.rdf".uri) --
FOAF.knows --> person }
When I come back to that code in a year having forgotten about it and
filled my head with other useless junk, it's easy to read in that one
line exactly what's happening, vs having to parse multiple lines of code
("where was the last import again?") to determine what's going on.
Perhaps that's not an issue in the specific domain you're trying to
solve here, but if that's the case I would say you're in a small minority.
Derek
Rex Kerr
Matthew Pocock said the following on 07/20/2011 06:21 PM:> there really is an anti-pattern here that has been identified --
> excessive and improper use of implicits that discard type safety.Imho implicit are a nice feature of scala bringing some of the advantages of dynamically typed languages to a static type-system. I don't see where type safety is discarded.
I agree. It's not that this code circumvents type-safety, but that the repeated importing of things
a) silently shadows other things in scopeb) injects hidden mutable state
Many names have been proposed for the alleged anti-pattern yet there is clearly no consensus what the anti-pattern is.
I think that at least Jim Powers, Matthew Pocock, and I all agree that it's the non-obvious context that is the problem. (Jim agreed explicitly, and Matthew's (a) and (b) together yield non-obvious context.) Whether this is specific enough to call an "anti-pattern", I don't know. Semantics aside, it's a bad idea.
If we invoke
println("hi")
we rely on the mutable field of System with a PrintStream proxied by System.out.
And if you keep pointing println to different places, I think this is the same anti-pattern (or bad design), and is to be avoided. Re-pointing println seems to me like a feature that was designed for people who don't have a good IDE that can refactor their printlns to x.println. If you use it for anything serious, in anything large, it's almost guaranteed to break because it's too easy to lose track of where it's pointing.
Similarly we act on an evironment in kojo if we draw a shape by sending around and changing the state of a turtle with
forward(100)
right()
penUp()
forward(100)
right()
penDown()
forward(100)
Yes, and if you have dozens of different consoles, that's a huge problem, especially if you use more than five or so different commands (at which point it's obvious that context is everything). Same bad design. And it's okay for the same reason that println is okay, in the same contexts: you normally only want to use one environment. (Also, I don't think kojo is intended for something serious like RESTful web services....)
On the clerezza console by default we act on the main content graph, entering the statement
zz>for (person <- FOAF.Person/-RDF.`type`) { "http://jibbering.com/foaf/santa.rdf".uri -- FOAF.knows --> person }
Would make sure that Santa knows everybody, where "everybody" is scoped to the world described by the content graph and this world is being changed. (Note that imho the smelliest part in the above is the uri method on string
Wait, what? This is classic Pimp-my-Library stuff. Why does this smell?
With Henry's proposal the above would look like this
zz>for (person <- contentGraph.node(FOAF.Person)/-RDF.`type`) { contentGraph.node("http://jibbering.com/foaf/santa.rdf".uri) -- FOAF.knows --> person }
Well, how many different contentGraphs are there? If vast, overwhelming majority of users have exactly one contentGraph, then this is a waste of typing. If two or more are common, then it looks reasonable. If you might want to interact with several at the same time or in quick proximity, then it's almost essential to do things this way.
The outcome of the statements depend crucially upon context which is not obvious, but which varies at least occasionally in typical code.
And here are two solutions:
(a) Provide context blocks that render the particular context more explcit
(b) Use a parameter and pass the context rather than relying upon it making its own way there
--Rex
Reto Bachmann-Gmür
> On 07/20/2011 01:06 PM, Reto Bachmann-Gmür wrote:
>> Functional programming avoids mutable states, but I don't see what's
>> particularly bad with "hidden" ones. Modification of a hidden mutable
>> state is like a side effect on the environment. If we invoke
>>
>> println("hi")
>>
>> we rely on the mutable field of System with a PrintStream proxied by
>> System.out.
> Ah, but you change that mutable state (which stream is being written to)
> via an explicit method call, not by an import. Using an import to modify
> state is what I believe to be the anti-pattern here.
System.setOut(pw1)
println("hello to pw1")
System.setOut(pw2)
println("hello to pw2")
While this, according to you, would be the discovered anti-pattern
import pw1.println
println("hello to pw1")
import pw2.println
println("hello to pw2")
While more powerful the first approach is also more dangerous and less
transparent. The state could be mutated by another thread and to find
out where the println ultimately goes to you have to scroll back the
code (at best, i.e. in a single threaded environment) while with the
second approach the switching of stream the println method is called
against is clear at compile time and an IDE could point you to the
respective import instruction (not that I know of one that would do
this, but its a potential advantage of the more static approach without
a mutable field).
> Having personally
> worked on a significant Scala codebase, I have to say that I would
> strongly prefer Henry's approach that you outlined:
>
>> for (person <- contentGraph.node(FOAF.Person)/-RDF.`type`) {
> contentGraph.node("http://jibbering.com/foaf/santa.rdf".uri) --
> FOAF.knows --> person }
>
> When I come back to that code in a year having forgotten about it and
> filled my head with other useless junk, it's easy to read in that one
> line exactly what's happening, vs having to parse multiple lines of code
> ("where was the last import again?") to determine what's going on.
> Perhaps that's not an issue in the specific domain you're trying to
> solve here, but if that's the case I would say you're in a small minority.
The example line was an actual command on the clerezza console, where
the needed imports are there by default (i.e. by a shell-customizer
provided by the clerezza platform). So in this concrete case looking
back at the code in a year is not an issue: you're entering code to
change the current state (triples/world view) of the platform, the line
on the clerezza shell for instance changes the triples served by
clerezza as a linked data server
As for code that will be maintained in future it's as I said a question
of style an priority. You're IDE could support you finding the right
import, you could choose to have the imports only at the beginning of
blocks or you can choose not to have the import at all and use the style
Henry's patch would enforce.
Reto
Reto Bachmann-Gmür
I think that at least Jim Powers, Matthew Pocock, and I all agree that it's the non-obvious context that is the problem. (Jim agreed explicitly, and Matthew's (a) and (b) together yield non-obvious context.) Whether this is specific enough to call an "anti-pattern", I don't know. Semantics aside, it's a bad idea.
(Note that imho the smelliest part in the above is the uri method on string
Wait, what? This is classic Pimp-my-Library stuff. Why does this smell?
With Henry's proposal the above would look like this
zz>for (person <- contentGraph.node(FOAF.Person)/-RDF.`type`) { contentGraph.node("http://jibbering.com/foaf/santa.rdf".uri) -- FOAF.knows --> person }
Well, how many different contentGraphs are there? If vast, overwhelming majority of users have exactly one contentGraph, then this is a waste of typing. If two or more are common, then it looks reasonable. If you might want to interact with several at the same time or in quick proximity, then it's almost essential to do things this way.
This is an example where we copy triples from another graph to the content graph (runs like that on a pristine clerezza instance):
zz>import platform.graphnodeprovider.GraphNodeProvider
zz>val gnp = $[GraphNodeProvider]
zz>val santa = gnp.get("http://identi.ca/user/27087".uri)
zz>for (p <- santa/FOAF.knows) { p.getNode -- RDF.`type`--> FOAF.Person }
The above instructions cause the clerezza instance to believe (i.e. to have the assertions in the content graph) that any resource foaf:known by Santa is a foaf:Person. The .getNode method call is needed to detach the node from the original context (Santa's profile).
I agree that the following variant (that works with current clerezza) is easier to understand
zz>for (p <- santa/FOAF.knows) {contentGraphPreamble.toRichGraphNode(p.getNode) -- RDF.`type`--> FOAF.Person }
So, here's the anti-pattern:
The outcome of the statements depend crucially upon context which is not obvious, but which varies at least occasionally in typical code.
And here are two solutions:
(a) Provide context blocks that render the particular context more explcit
(b) Use a parameter and pass the context rather than relying upon it making its own way there
What about imports in general, import collection.mutable._ has an influence on the outcome of subsequent statements, is it crucial enough to fall under the anti pattern?
On the clerezza console entering
zz>import out._
will cause subsequent println-statements to print to the current console (rather than System.out which is typically unavailable when running as service), would this fall under the anti-pattern?
And to come back to the original example showing the code the anti-pattern in a way "obvious to anyone who looks at it", is the anti-pattern in the library the code uses, or is the anti-pattern just in the example code?
I was looking for files in clerezza that actually use different graph contexts. From the various files actually using the library I found none using different contexts in the trunk. I found one in an issue branch:
https://svn.apache.org/repos/asf/incubator/clerezza/issues/CLEREZZA-388/rdf.cris/core/src/main/scala/org/apache/clerezza/rdf/cris/GraphIndexer.scala
in this file in one block there is
import definitionsPreamble._
and in another
import basePreamble._
If that is considered to be confusing we could make the method calls explicit there and drop the imports without changing the library and requiring more typing in the situations where only one context is relevant.
Cheers,
Reto
Tony Morris
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 20/07/11 00:26, Henry Story wrote:
> variable
>
Guys guys, chill out, I found the problem.
- --
Tony Morris
http://tmorris.net/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk4n8X8ACgkQmnpgrYe6r63BMACfUvY85kmlb07cMAPMfN1S1jcv
J8UAoJt2EjqTflb/m0FOMjgHKEp7QM0r
=p+hj
-----END PGP SIGNATURE-----
√iktor Ҡlang
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 20/07/11 00:26, Henry Story wrote:
> variable
>
Guys guys, chill out, I found the problem.
ROFL
- --
Tony Morris
http://tmorris.net/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAk4n8X8ACgkQmnpgrYe6r63BMACfUvY85kmlb07cMAPMfN1S1jcv
J8UAoJt2EjqTflb/m0FOMjgHKEp7QM0r
=p+hj
-----END PGP SIGNATURE-----
--
Viktor Klang
Akka Tech Lead
Reto Bachmann-Gmür
> val graphB3 = new EzMGraph() {
> node(uriA) -- FOAF.knows --> ( uriC -- FOAF.knows --> uriD )
> }
A few questions and comments to this proposal:
1. How comes you don't need to invoke node for uriC?
2. Is the -->-method overloaded to allow both wrapped and unwrapped
resources as argument?
3. to avoid the explicit node call a protected implicit method would
be enough, this wouldn't allow constructing the ugly strawman you
started the thread with, what's bad with this?
4. I think the required different handling of subject and object
defeats the goal of reaching similarity with dedicated
RDF-serialization formats: uri("http://bblfish.net/#hjs") --
FOAF.homePage --> "http://bblfish.net/#hjs".uri
Reto
Henry Story
On 21 Jul 2011, at 11:44, Reto Bachmann-Gmür wrote:
> On Wed, Jul 20, 2011 at 4:10 PM, Henry Story <henry...@gmail.com> wrote:
>> val graphB3 = new EzMGraph() {
>> node(uriA) -- FOAF.knows --> ( uriC -- FOAF.knows --> uriD )
>> }
>
> A few questions and comments to this proposal:
>
> 1. How comes you don't need to invoke node for uriC?
In the previous commit [1] I added an implicit
implicit def resourceToRichGraphNode(res: Resource) = new RichGraphNode(res, new SimpleMGraph())
to the Preamble. That make senses, especially for cases outside of an EzMGraph.
But it is true that this is a bit clumsy. One would wish to also have resource inside an EzMGraph
automatically converted to RichGraphNodes with the Graph as the store. So I added a new patch [2] right
now
implicit def resourceToRichGraphNode = null
implicit def node(resource: Resource) = new RichGraphNode(resource,baseTc)
This cancels the general Preamble implicit as explained by Josh Suereth in his
presentation on implicits [3] slide 17, and it allows one to write something very cleanly
val graphA4 = new EzMGraph() {
uriA -- FOAF.knows --> ( uriB -- FOAF.name --> "Dan Brickley" )
}
Now of course one is what you had initially.
It is close to what I was calling the anti-pattern. Because it is possible from there on to call
export graphA4.node
One could perhaps protect that implicit so it is only seen by subclasses.
> 2. Is the -->-method overloaded to allow both wrapped and unwrapped
> resources as argument?
It allows both a resource and a RichGraphNode. That code has not changed as
and can be seen on github. In the example above it takes a GraphNode, and with the
implicit it does indeed give it the same graph as the containing EzMGraph (I think)
>
> 3. to avoid the explicit node call a protected implicit method would
> be enough, this wouldn't allow constructing the ugly strawman you
> started the thread with, what's bad with this?
yes. That is what I was coming to the conclusion of.
The implicit inside the class seems to work, and it could be protected.
As shown above it seems to be possible to have a local implicit that uses the local tcgraph,
and a more generic implicit that would work outside of the graph context by constructing a
mutable graph.
>
> 4. I think the required different handling of subject and object
> defeats the goal of reaching similarity with dedicated
> RDF-serialization formats: uri("http://bblfish.net/#hjs") --
> FOAF.homePage --> "http://bblfish.net/#hjs".uri
You mean .uri and uri() ?
Yes, that's another issue. I am not religious about it. Other things I had thought of
was an
object uri { def apply(uriStr: Uri) = new UriRef(uriStr) }
-----
Anyway, it is true that a graph of statements is very close to what in Scala one would
have inside a { ... }. It is a context for writing something. Within such a context one
can argue that any other context that appears needs to be placed inside its own { }.
So perhaps the import ez.node is not such an impossible thing.
But at the same time one should perhaps not be forcing that either on the user. Because there
may well be cases where one is filtering statements into various graphs, and so be working
with many simultaneously.
Ideally one would wish this to be quite explicit with someting like
graph g { a -- rel --> b
-- re2 --> ( bnode -- re3 --> something )
}
Subclassing the graph seems to come closest to that.
Henry
>
> Reto
[1] https://github.com/bblfish/clerezza/commit/51be3b14a5488838d87094e81455ca806507b72b
[2] https://github.com/bblfish/clerezza/commit/560bc117b5ae38151aa53205f51743d68705d65a
[3] http://scala-lift.blogspot.com/2011/03/josh-suereth-on-implicits-without.html
I have not yet finished watching, so I may still discover all kinds of things.
Rex Kerr
Rex Kerr said the following on 07/20/2011 11:46 PM:
(Note that imho the smelliest part in the above is the uri method on string
Wait, what? This is classic Pimp-my-Library stuff. Why does this smell?
Because String is a very generic and common class, multiple libraries may want to add a uri method to it (see Josh Suereth interesting presentation)
Then you should use one such library at the same time, or call the converter explicitly if you've got several. String to regex conversion is another case like that--it does the obvious thing, so it's a good idea to include it.
Now, if the conversion does something _other_ that convert the string to the obvious corresponding uri, I would agree with you that it's a smell. A pimped .parse method that took an implicit parser would be a bad idea, for example, because there is no obvious "default parse" for a string.
zz>val gnp = $[GraphNodeProvider]
Wait, you're using $ as a method name? Don't you think that's unwise, given that $ is used as a special identifier character in name mangling?
So, here's the anti-pattern:The outcome of the statements depend crucially upon context which is not obvious, but which varies at least occasionally in typical code.
And here are two solutions:
(a) Provide context blocks that render the particular context more explcit
(b) Use a parameter and pass the context rather than relying upon it making its own way there
Thank you for giving a proposal what the too-much-pimpin/pimpin-reto/implicit-cat/gimp-my-library/implicit-aroused/name-shadowing-2.0 anti-pattern actually is about. The terms "crucially" and "obvious" seem however problematic. An implicit argument for instance is always non-obvious (at least less obvious than having it explicit), should this thus only be used for argument that have no "crucial" influence on the outcome?
You overlooked the "but" part of my sentence. I put it there for a reason.
The prompt shown to the user might be a non crucial influence of a command the implict ordering argument on List.sorted however has a crucial impact on the result.
If you are changing your ordering frequently, you should specify the ordering explicitly, not allow the implicit to determine the order. Otherwise, people will assume that the sort is in the default order when it is not.
What about imports in general, import collection.mutable._ has an influence on the outcome of subsequent statements, is it crucial enough to fall under the anti pattern?
collections.mutable and collections.immutable are designed to be drop-in replacements. Although you get extra methods for mutable collections, the outcome is the same if you use the methods defined on immutable collections. In those cases where this was not true, it was recognized as a bug and the library was changed (or at least the old behavior was deprecated). Otherwise, yes, this would be an example.
On the clerezza console entering
zz>import out._
will cause subsequent println-statements to print to the current console (rather than System.out which is typically unavailable when running as service), would this fall under the anti-pattern?
This is fine if System.out really is unavailable, since the context is "println doesn't work". If you see println used, you should know what's going on. (I assume you're just shadowing the println name, not redirecting the stream.)
And to come back to the original example showing the code the anti-pattern in a way "obvious to anyone who looks at it", is the anti-pattern in the library the code uses, or is the anti-pattern just in the example code?
I don't know the code well enough to comment. In general, one can assume that people coding libraries understand the library more deeply than users of the libraries, so things that might be confusing to a novice won't be as problematic.
I was looking for files in clerezza that actually use different graph contexts. From the various files actually using the library I found none using different contexts in the trunk. I found one in an issue branch:
https://svn.apache.org/repos/asf/incubator/clerezza/issues/CLEREZZA-388/rdf.cris/core/src/main/scala/org/apache/clerezza/rdf/cris/GraphIndexer.scala
in this file in one block there is
import definitionsPreamble._
and in another
import basePreamble._
I don't know the library, but the names "basePreamble" and "definitionsPreamble" don't scream out to me "different graph contexts!".
But anyway, I am less concerned about the library than about standard use cases. Would anyone want to use different graphs? If yes, selecting them based on sort-of-hidden context is unwise. Henry's example made it look like using different graphs was a pretty reasonable thing to do. And note that a one-character variable is a lot more informative than a zero-variable character:
graphContext(definitionsPreamble.currentGraph) { g =>
whatever -- g(edgelabel) --> soandso
}
Now you have, with a couple extra characters per call, documentation of what the relevant context is.
P.S. I wouldn't personally have used the broken arrow that way; I'd have gone for something like g(edge) = whatever -> soandso. That reduces the number of specialized tokens and isn't any less clear. (At least I hope I've understood correctly that the thing in the middle of the arrow is the edge label.)
Henry Story
P.S. I wouldn't personally have used the broken arrow that way; I'd have gone for something like g(edge) = whatever -> soandso. That reduces the number of specialized tokens and isn't any less clear. (At least I hope I've understood correctly that the thing in the middle of the arrow is the edge label.)
Henry Story
But anyway, I am less concerned about the library than about standard use cases. Would anyone want to use different graphs? If yes, selecting them based on sort-of-hidden context is unwise. Henry's example made it look like using different graphs was a pretty reasonable thing to do. And note that a one-character variable is a lot more informative than a zero-variable character:
graphContext(definitionsPreamble.currentGraph) { g =>
whatever -- g(edgelabel) --> soandso
}
Now you have, with a couple extra characters per call, documentation of what the relevant context is.
Henry Story
Stefan Langer
it will look a lot cleaner to my eyes.
-
Stefan
2011/7/22 Henry Story <henry...@gmail.com>:
Henry Story
On 22 Jul 2011, at 13:16, Stefan Langer wrote:
> Create a factory method and new context will just become context and
> it will look a lot cleaner to my eyes.
I tried something like that, but I don't think that is possible in this case.
I tried it by creating an
object context {
def apply(graph: MGraph)(body: ??) = ??
}
but if one puts something like
context(g) { uriA -- FOAF.knows --> ( bnode("joe") -- FOAF.knows --> uriD ) }
Then because the body is not subclassing the context class it won't be able to find the
context.bnode method, nor will it be able to convert the uriD to the appropriate GraphNode
by using the implicit node(..) method ( see http://bit.ly/nyy7IF )
class context(val graph: MGraph) { ...
protected implicit def node(resource: Resource) = new ContextGraphNode(resource)
protected def bnode = new ContextGraphNode(new BNode)
protected class ContextGraphNode(resource: Resource) extends RichGraphNode(resource,graph) { ... }
}
So I can't yet see a substitute to subclassing right now. It looks like subclassing captures the
notion of context that we need. But it is quite likely that there is some scala magic I don't know
about yet :-)
Henry
Reto Bachmann-Gmür
the clerezza rdf library.
Importing the members of an EzGraph as context makes perfectly sense
on the clerezza console. The "coding" there is imperative, one want to
see the results ideally after each line. Creating a block scoped to
graph is clearly much less user-friendly [1]. Also having the content
graph as default is very useful and shouldn't be sacrificed for
enforcing good design in another context. Opening and being in the
context you most often want to be is not only handy for rapid queries,
the shell is a triple sink where new triples can be added to the
clerezza instance simply by writing them down in a syntax similar to
the turtle or the n-triples rdf format. Not having to call a method on
some nodes on the graph makes a big difference in terms of usability.
In program code that as opposed to the commands on the shell is
deigned to last and which isn't executed while you type it is better
design practice to instantiate an anonymous subclass of EzGraph (which
could be renamed or aliased as context as in your examples) rather
than importing the members (methods and implicts) of an instance.
Your anti-pattern example shows horrible code that you can write using
the library, you could very well write a similar example using the
java transaction api. The clerezza API however supports better coding
practices and the documentation shall focus on these, changing the api
to enforce these better practice would be at high costs for those
using the api interactively on the shell or in scripts.
Reto
1. the clerezza faq entry on how to reset the password shows an
example real-life usecase for switching graph-context on the console
Rex Kerr
--Rex
Lex
accepted, not a big deal. If you feel a strong disagreement with the
author, just fork the project. If you are unwilling to maintain the
fork, then think about how much effort it takes to start a project and
keep it going. Maybe that will make you more respectful and
professional about the subject.
Banging on the chest and pointing to your way of doing things as
superior, while calling the original "anti-pattern" wont get you
anywhere.
Henry Story
b_("reto") -- FOAF.homepage --> "http://bblfish.net/".uri
import ez2._
b_("hjs") -- FOAF.homepage --> "http://bblfish.net/".uri
-- name --> "Lex"
-- knows --> ( bnode("hjs") -- name --> "Henry Story")
-- knows --> ( reto -- knows -- bnode("hjs"))
)
-- name --> "Lex"
-- knows --> ( bnode("hjs") -- name --> "Henry Story")
-- knows --> ( reto -- knows -- bnode("hjs"))
)}
Does this really belong on the scala list?
So your patch was not accepted, not a big deal. I
f you feel a strong disagreement with the
author, just fork the project.
If you are unwilling to maintain the fork, then think about how much effort it takes to start a project and
keep it going. Maybe that will make you more respectful and
professional about the subject.
Banging on the chest and pointing to your way of doing things as
superior, while calling the original "anti-pattern" wont get you
anywhere.