pchampin / sophia_rs

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

Add `Debug` supertype to `iri` parameter in `get` function of `Loader` trait #169

Open GordianDziwis opened 1 month ago

GordianDziwis commented 1 month ago
impl Loader for ResourceLoader {
    fn get<T: Borrow<str>>(
        &self,
        iri: Iri<T>,
    ) -> std::result::Result<(Vec<u8>, String), LoaderError> {
    iri.borrow_term() // This is not possible because T does not implment Debug
    iri.clone() // This is also not possible, because T does not implement Clone
...
}
pchampin commented 1 month ago

Just to clarify: your suggestion is to change the declaration of Loader::get from

fn get<T: Borrow<str>>(
        &self,
        iri: Iri<T>
    ) -> Result<(Vec<u8>, String), LoaderError>;

to

fn get<T: Borrow<str> + Debug>(  // <-- added "+ Debug" here
        &self,
        iri: Iri<T>
    ) -> Result<(Vec<u8>, String), LoaderError>;

right?

I'm guessing that you are encountering issues when generating a log or error message, because something like format!("{:?}", iri) does not work?

Given that Iri<T> can easily be converted to strings, I think you can actually do without the Debug trait: format!("<{}>", iri.as_str() would give an even better message (in terms of user-friendliness, at least).

GordianDziwis commented 4 weeks ago

Addung either Debug or Clone is fine.

I needed just two owned values of iri.

pchampin commented 4 weeks ago

Sorry, I think I initially read the body of your issue too quickly.

The immediate solution to your problem is to use Iri::as_ref, which gives you a "copy" of the IRI, borrowing data from the original.

I understand that it was tempting to use Term::borrow_term instead, and indeed, it is surprising that Iri<T> does not implement Term in this case (even I was surprised :stuck_out_tongue: ). I just opened #170 to remove this surprising exception.