You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@clerezza.apache.org by Henry Story <he...@bblfish.net> on 2011/03/16 12:20:45 UTC
Re: RichGraphNode
On 15 Mar 2011, at 20:16, Reto Bachmann-Gmuer wrote:
>
>
> On Tue, Mar 15, 2011 at 8:13 PM, Henry Story <he...@bblfish.net> wrote:
> Those are questions I was asking when looking at the code. Can you answer them?
> I just added a new constructor, because the given one was a bit lame. But I did not want to replace the one without understanding more. Hence the comments below.
> I reposted on mailing list as originally intended.
>
> My asking back to your questions is inline.
>
> Reto
>
>
> Henry
>
> On 15 Mar 2011, at 20:02, Reto Bachmann-Gmuer wrote:
>
>> Hi Henry
>>
>> Could you please fix yesterday's commit? There are still experiments and discussion in it.
>>
>> One discussion (taken from rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala):
>>
>> /* because it is tedious to wrap nodes as happens in a lot of code.
>>
>> That's why there are dynamic conversions, the Preamble Object converts any GraphNode to RichGraphNode, instances of the preamble class also convert Resources (such as UriRefs) to RichGraphNodes.
Perhaps, but then sometimes the new constructor is easier than an import statement.
I found it a bit odd in some code to create an import statement for this.
I think one should not use dynamic conversions unnecessarily. They should be used carefully when needed.
>> * todo: does one really need to create the graph node? Is there a reason this is passed ike that,
>> What do you suggest, RichGraphNode not to be a subclass of GraphNode?
No, it was just a question there I had not time to answer.
>>
>> * todo: or was that just a quick hack? If it is because we don't want to use any of the superclass implementations
>> Not sure what you're refreing too, RichGraphNode doesn't reimplement any of the superclass methods
I was just wondering why for example
def /(property: UriRef) = {
new CollectedIter(() => new GraphNodeIter(node.getObjects(property)), readLock)
}
calls node.getObjects() and not just getObjects() since it is extending the class.
There could be reasons for calling it on node, if one felt that it could contain behavior that was important. But then the other methods from the superclass would not be calling it. So that seems wrong.
>> * todo: then it would be worth creating an interface above GraphNode and implementing the interface instead...
>> I see no need for this.
Probably not no. I was just trying to understand what was going on here.
As there is no documentation I put my thoughts in a todo.
Next step is to document the class....
>>
>>
>> Cheers,
>> reto
>>
>>
>
> Social Web Architect
> http://bblfish.net/
>
>
Social Web Architect
http://bblfish.net/
Re: RichGraphNode
Posted by Henry Story <he...@bblfish.net>.
On 16 Mar 2011, at 20:51, Reto Bachmann-Gmuer wrote:
>
> Perhaps, but then sometimes the new constructor is easier than an import statement.
> I found it a bit odd in some code to create an import statement for this.
> I didn't notice the auxiliary constructor wasn't there before. It's certainly good to have, but as RichGraphNode is a decorator for GraphNode the primary constructor should remain as it is.
Ok so the reason I put those "TODOs" in there which should perhaps have been QUESTIONS:
is that It seems to be silly to have the primary constructor be one that requires one to create an object that then gets deconstructed, as we see here:
class RichGraphNode(node: GraphNode) extends GraphNode(node.getNode, node.getGraph) {
because if that is the primary constructor then even a secondary constructor that took a node and a graph would need to pass that to the primary constructor, which would then immediately discard the created object. Rather it would be better to have
class RichGraphNode(node, graph) extends GraphNode(node, graph) {
def this(node: GraphNode) = this(node.getNode, node.getGraph)
...
}
>
> Next step is to document the class....
> If you're TODOs indicate that there will be a documentation very soon, then I've less strong feelings against them ;)
Yes, I was just trying to understand the code...
Henry
Social Web Architect
http://bblfish.net/
Re: RichGraphNode
Posted by Reto Bachmann-Gmuer <re...@trialox.org>.
On Wed, Mar 16, 2011 at 12:20 PM, Henry Story <he...@bblfish.net>wrote:
>
> On 15 Mar 2011, at 20:16, Reto Bachmann-Gmuer wrote:
>
> >
> >
> > On Tue, Mar 15, 2011 at 8:13 PM, Henry Story <he...@bblfish.net>
> wrote:
> > Those are questions I was asking when looking at the code. Can you answer
> them?
> > I just added a new constructor, because the given one was a bit lame. But
> I did not want to replace the one without understanding more. Hence the
> comments below.
> > I reposted on mailing list as originally intended.
> >
> > My asking back to your questions is inline.
> >
> > Reto
> >
> >
> > Henry
> >
> > On 15 Mar 2011, at 20:02, Reto Bachmann-Gmuer wrote:
> >
> >> Hi Henry
> >>
> >> Could you please fix yesterday's commit? There are still experiments and
> discussion in it.
> >>
> >> One discussion (taken from
> rdf.scala.utils/src/main/scala/org/apache/clerezza/rdf/scala/utils/RichGraphNode.scala):
> >>
> >> /* because it is tedious to wrap nodes as happens in a lot of code.
> >>
> >> That's why there are dynamic conversions, the Preamble Object converts
> any GraphNode to RichGraphNode, instances of the preamble class also convert
> Resources (such as UriRefs) to RichGraphNodes.
>
> Perhaps, but then sometimes the new constructor is easier than an import
> statement.
> I found it a bit odd in some code to create an import statement for this.
>
I didn't notice the auxiliary constructor wasn't there before. It's
certainly good to have, but as RichGraphNode is a decorator for GraphNode
the primary constructor should remain as it is.
> I think one should not use dynamic conversions unnecessarily. They should
> be used carefully when needed.
>
>
> >> * todo: does one really need to create the graph node? Is there
> a reason this is passed ike that,
> >> What do you suggest, RichGraphNode not to be a subclass of GraphNode?
>
> No, it was just a question there I had not time to answer.
>
>
> >>
> >> * todo: or was that just a quick hack? If it is because we
> don't want to use any of the superclass implementations
> >> Not sure what you're refreing too, RichGraphNode doesn't reimplement any
> of the superclass methods
>
> I was just wondering why for example
>
> def /(property: UriRef) = {
> new CollectedIter(() => new
> GraphNodeIter(node.getObjects(property)), readLock)
> }
>
> calls node.getObjects() and not just getObjects() since it is extending the
> class.
> There could be reasons for calling it on node, if one felt that it could
> contain behavior that was important. But then the other methods from the
> superclass would not be calling it. So that seems wrong.
>
I see no particular reason, but no harm either.
>
>
> >> * todo: then it would be worth creating an interface above
> GraphNode and implementing the interface instead...
> >> I see no need for this.
>
> Probably not no. I was just trying to understand what was going on here.
> As there is no documentation I put my thoughts in a todo.
>
> Next step is to document the class....
>
If you're TODOs indicate that there will be a documentation very soon, then
I've less strong feelings against them ;)
Reto