changed in error handling and iterator semantics #27
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes the error-handling of sophia and some semantics of
XSource
andXSink
traits.I know, I shouldn't do two different things in one PR but I couldn't resist ...
Change in error-handling
This PR canges the error-handling in
sophia
fromerror-chain
andcoercible-errors
tothiserror
andanyhow
.Specific errors
As mentioned in #8 is the
error-chain
-crate somewhat outdated. I choose the relatively newthiserror
as it is very simple and easy to use.In addition the big
sophia::error::Error
is split into module-errors as suggested bysnafu
. This will make it easier to split upsophia
(#26) and reduces dependencies between the modules. Furthermore, it is someaht more aligned with the philosophie of "each implementation can provide its own error".Coliding error-types
This was handled by
coercible-errors
before and is changed toanyhow
. It providesanyhow::Error
which is "a betterBox<dyn Error>
". It can be created from anytype that implementsstd::error::Error
, therefore, is suitable when different error-types colide.As stated in
anyhow
's docs cananyhow::Error
be downcasted to a concrete error-type, so users can still match against concrete types, e.g. done in the parser test.I don't want to offend
coercible-errors
. I just think the handling of many different errors will become more and more complicated as thesophia
-ecosystem grows as implementers probably provide their own error-types. The complexity has been presented atgraph::Graph::retain()
which had a bizzar and complicated trait bound and now just returnsanyhow::Error
.The merge-everything
anyhow::Error
is only used when two or more errors are used within one function. Everything else returns a specific error-type. This highlights better where the user has to be aware of error-mixing.Semantics of
Iterator
sWhen "Triples" are mentioned in the following section they can always be substituted with "Quads"
The semantics of
TripleSource
andTripleSink
always felt a bit cumbersome to me. As a result, I changed the semantics of them a bit.Consume sources
Often
Iterator
s where past in by&mut
butIterator
s are designed to be consumed by iteration, therefore, it is more idomatic to move them into a function. Accordingly, I addedTripleSink::feed_all()
method that takes aTripleSource
.The semantic of a
TripleSink
is also not totaly clear to me. On the one hand there is the 'serializer'-sink that consumes and finishes just likeTripleSource
's API suggests. On the other hand there are 'graph'-sinks that consume triples as-well but finishing them seems a bit unnatural. As a consequence, I removed theGraph::inserter
andGraph::remover
methods which are equally substituted byGraph::insert_all
andGraph::remove_all
.Create from TripleSource
The builder-pattern for graphs seemed a bit unidomatic for me. Consequently, I removed
in_sink
andin_graph
methods and created the newCollectToGraph
-trait which extends the standardIterator
. I think this is more accesible for Rust programmers as its more close to the standard API (cf. getting started example inlib.rs
). I tried to implement something likeimpl FromIterator for Result<Graph>
so user could just callcollect()?
on aTripleSource
but this is vorbidden by Rust's orphan rule...Conclusion
I'm totally aware that these are huge changes and that you might think that some of them are to much. Most of the changes I did are for sake of better ergonomics and a more idomatic Rust. I am open to disucussion and l will gladly change the PR if required.