[ 
https://issues.apache.org/jira/browse/COMMONSRDF-46?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15614998#comment-15614998
 ] 

ASF GitHub Bot commented on COMMONSRDF-46:
------------------------------------------

Github user stain commented on a diff in the pull request:

    https://github.com/apache/incubator-commonsrdf/pull/25#discussion_r85505133
  
    --- Diff: 
rdf4j/src/test/java/org/apache/commons/rdf/rdf4j/NativeStoreGraphTest.java ---
    @@ -49,17 +51,23 @@
      */
     public class NativeStoreGraphTest extends AbstractGraphTest {
     
    -   public final class NativeStoreFactory implements RDFTermFactory {
    +   public final class NativeStoreRDF implements RDF {
     
    -           RDF4JFactory rdf4jFactory = new 
RDF4JFactory(getRepository().getValueFactory());
    +           RDF4J rdf4jFactory = new 
RDF4J(getRepository().getValueFactory());
     
                @Override
                public RDF4JGraph createGraph() {
                        // We re-use the repository connection, but use a 
different context every time
                        Set<RDF4JBlankNode> context = 
Collections.singleton(rdf4jFactory.createBlankNode());
                        return rdf4jFactory.asGraph(getRepository(), context);
                }
    -
    +           @Override
    +           public Dataset createDataset() {
    +                   throw new UnsupportedOperationException("Can't create 
more than one Dataset in this test");
    --- End diff --
    
    I tried to do that, yes. The problem then was that you couldn't easily see 
what caused the 30s delay if something was not properly closed. Now that 
everything is closed properly within the tests it doesn't take many seconds to 
run all of the tests.


> Rename RDFTermFactory to RDF
> ----------------------------
>
>                 Key: COMMONSRDF-46
>                 URL: https://issues.apache.org/jira/browse/COMMONSRDF-46
>             Project: Apache Commons RDF
>          Issue Type: Bug
>          Components: api
>            Reporter: Stian Soiland-Reyes
>            Assignee: Stian Soiland-Reyes
>             Fix For: 0.3.0
>
>
> As [mentioned on 
> dev@commons|https://lists.apache.org/thread.html/ff9f0eda82a70fea38bd46781a062d182cd7792aee57a4563f854b27@%3Cdev.commonsrdf.apache.org%3E],
>  the {{RDFTermFactory}} will grow in 0.3.0 to include {{Dataset}} and 
> {{Quad}} creation, which are not {{RDFTerm}} instances.
> As well, the new implementations of RDFTermFactory for Jena and RDF4J also 
> include converter methods from/to their underlying types - which feel 
> somewhat wrong in a "factory" as they may are free to wrap/unwrap rather than 
> make new instances. 
> So the suggestion is a radical style change - rename {{RDFTermFactory}} to 
> {{RDF}}, and its children to {{SimpleRDF}} {{JenaRDF}}, {{RDF4J}}, 
> {{JsonLDRDF}}.
> Typical usage then looks pretty neat:
> {code}
> RDF rdf = new JenaRDF(); 
> IRI iri = rdf.createIRI("http://example.com/";); 
> Triple triple = rdf.createTriple(iri, iri, iri); 
> Graph graph = rdf.createGraph(); 
> graph.add(triple);
> {code}
> but works less well as a static constant {{RDF}}:
> {code}
> private static final RDF RDF = new JenaRDF();
> {code}
> (before {{FACTORY}} might have made sense)
> Some style considerations:  
> * {{RDF4JRDF}} looks weird, so just {{RDF4J}} there
> * {{SimpleRDF}} looks good (as Simple does not exists outside Commons RDF)
> * Jena already have 
> [org.apache.jena](https://jena.apache.org/documentation/javadoc/jena/org/apache/jena/Jena.html),
>  so {{JenaRDF}} is better than another {{Jena}}
> * {{JsonLdRDF}} 
> * Documentation about just {{RDF}} the interface can be confusing against 
> _RDF_ the concept, requiring using {{<code>}}-style typography and expanded 
> phrases like "an {{RDF}} implementation" instead of "an {{RDF}}"
> A milder variant is: {{RDFFactory}} with children {{SimpleRDFFactory}}, 
> {{JenaFactory}}, {{RDF4JFactory}}. {{JsonLDFactory}} -- here we can skip 
> {{RDF}} from the children except from the newbie {{Simple}}.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to