Use Jena's notion of graph containment instead of our strict equality #552
Conversation
cbeer
force-pushed
the
jena-graph-containment
branch
from
878b08e
to
9f3e4f0
Compare
cbeer
force-pushed
the
jena-graph-containment
branch
from
9f3e4f0
to
20530a4
Compare
| @@ -82,8 +97,8 @@ protected E computeNext() { | |||
| * | |||
| * @return The elements that turned out to be common to the two inputs. | |||
| */ | |||
| public Set<E> common() { | |||
| return source.hasNext() ? null : common; | |||
| public Iterator<Triple> common() { | |||
There was a problem hiding this comment.
What's the advantage of returning the iterator here? Why not admit that the triples are in-heap and return the Graph? Are we just trying to avoid Jena types, 'cause I think it's too late for that... {grin}
There was a problem hiding this comment.
In the previous iteration of this commit, I thought you found Graph#find(null, null, null) in the FedoraResourceImpl distasteful.
There was a problem hiding this comment.
Yeah, sorry to be unclear-- I was hoping for aCollection<Triple> here, because that makes it clear to the client that
- this is in-memory stuff, with the attendant risks, and
- you can iterate on it as much as you like.
There was a problem hiding this comment.
I'm pretty sure that's not part of the Graph contract (because it's possible to not have a whole Graph in memory?).
There was a problem hiding this comment.
Yeah, I'm saying we actually fold up the Graph and return a non-Jena type. But this is, again, a question of just how much we're going to fight the Jena types percolating through our system. Maybe it's better to just go with this as it is, since I am going to come back at some point soon and factor equivalence out of this type anyway. That might be the time to worry more about it. {sigh}
There was a problem hiding this comment.
I'd leave it for now; our one and only consumer then took the Set and turned it into an Iterator anyway.
Use Jena's notion of graph containment instead of our strict equality
https://www.pivotaltracker.com/story/show/80907094