Added Data.RDF.State for easier manipulation of graph with StateT Monad. #112
Conversation
src/Data/RDF/State.hs
Outdated
| addPrefixMappings | ||
| :: (Rdf rdfImpl, Monad m) | ||
| => PrefixMappings | ||
| -> Bool |
There was a problem hiding this comment.
I know this Bool maps to the underlying API but I would opt for distinct functions: one to add to existing PrefixMappings (this function) and another to replace the existing ones, say, replacePrefixMappings.
There was a problem hiding this comment.
@koslambrou This suggestion by @tmciver sounds good to me. Thoughts? Either way, haddock documentation to make the functionality clear would be great.
There was a problem hiding this comment.
Agreed! I'll change that
There was a problem hiding this comment.
Done. Also added a comment for each function pointing to the equivalent function in Data.RDF.Types.
src/Data/RDF/State.hs
Outdated
| -- -- remove one of my interests | ||
| -- RDFS.removeTriple triple1 | ||
| -- @ | ||
| type RdfST rdfImpl m a = StateT (RDF rdfImpl) m a |
There was a problem hiding this comment.
I'm on the fence about this idea but we might consider using a newtype wrapper here to hide use of StateT.
There was a problem hiding this comment.
Right! I'll use a newtype instead
There was a problem hiding this comment.
@tmciver Why are you on the fence exactly?
There was a problem hiding this comment.
Only because it doesn't buy a whole lot other than hiding the use of StateT. I think that's a good idea and I would use a newtype myself but it's hard to justify without a clear benefit. It was more an acknowledgement that not everyone would agree with this.
There was a problem hiding this comment.
Just converted to newtype. Is that as expected?
koslambrou
force-pushed
the
rdf-state-monad
branch
from
0f52a3a
to
ebb9896
Compare
|
@tmciver @robstewart57 Any thoughts on the changes? |
|
LGTM! |
|
@koslambrou LGTM also. Thanks very much! Merged. |
Solves issue #81