rio icon indicating copy to clipboard operation
rio copied to clipboard

Refactor the `Triple` variant of `Subject` and `Term`

Open pchampin opened this issue 3 years ago • 3 comments

Subject and Term both have a variant that goes:

    Triple(&'a Triple<'a>)

I am extending Sophia to use [TriplesFormatter], and I find the above very inconvenient. Everytime I want to create a Subject or Term "view" of my internal terms, if that term is a quoted triple, I need to allocate a Rio Triple somewhere that I can then reference. And I have to do this recursively, which is a pain. (I was unable to achieve that without some unsafe code...)

Did you run into a similar problem with Oxigraph? Did you find an elegant way to solve it?

I would suggest to change the variant in Subject and Term

    Triple(Box<Triple<'a>>)

I think that the code of the parser would be minimally impacted by that change, and it would make it much easier to use the formatters.

If you agree with the general idea, I can propose a PR.

pchampin avatar Sep 02 '22 07:09 pchampin

Did you run into a similar problem with Oxigraph? Did you find an elegant way to solve it?

Yes, I ran into a similar problem and I have not found a way to elegantly solve it. I use Triple(Box<Triple<'a>>) just like you suggested inside of oxrdf.

If you agree with the general idea, I can propose a PR.

Yes, feel free to do so.

Tpt avatar Sep 02 '22 07:09 Tpt

Just gave it a quick try, but unfortunately, this change is much more disruptive than I anticipated...

  • if we go that way, Subject and Term can not implement Copy anymore,
  • so Triple and Quad can't implement Copy either,
  • but the whole API assumes that Triple and Quad are copy, because the on_triple method passed to TriplesParser expects a Triple passed by value, not by reference (and similarly for QuadParser).

This would force implementation of TriplesParser to clone the triple before passing it to on_triple, possibly causing recursive clones of the boxed quoted triples, which would go against all the optimizations done by Rio for atomic terms...

pchampin avatar Oct 18 '22 09:10 pchampin

Thank you for trying!

I am moving forward well with my new N3/Turtle/N-Triples parser. It seems to be at least twice as fast as Rio. So, if you are fine to wait a bit I hope to release it in the next few month.

Tpt avatar Oct 18 '22 10:10 Tpt