search

pchampin / sophia_rs

Sophia: a Rust toolkit for RDF and Linked Data
Other
210 stars 23 forks source link

simplify the Resolve trait implementation #65

Closed pchampin closed 4 years ago

pchampin commented 4 years ago

This commit remove somewhat redundant implementations; the rationale is that what they did (resolving TD1 to TD2 directly) can usually be achieved by resolving to MownStr, then map'ing to TD2. This will usually be just as efficient (possibly more), because types that can recycle the memory allocated by MownStr will usually do so (e.g. Box or String).

@MattesWhite what do you think about that. You designed and implemented the Resolve trait, so I would like your opinion on this before merging it.

MattesWhite commented 4 years ago

Nice work 👍 I think this simplification will realy increase the usability of sophia.

Inspecting the _join.rs I found the implementation of Resolve<Literal> a little bit akward. I think this could easily optimzed as it calls dt.value().as_ref()... maybe this could be improved as part of the PR. Something like matching on the literal's kind and then use self.resolve(dt) or so.

pchampin commented 4 years ago

I agree that this is ugly, but I couldn't figure a way to make it work by simply resolving other.dt(). Lifetime hell strikes again... I don't have the bandwidth to address it right now, but fell free...