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

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

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

    https://github.com/apache/incubator-commonsrdf/pull/25#discussion_r85231845
  
    --- Diff: api/src/main/java/org/apache/commons/rdf/api/RDFSyntax.java ---
    @@ -31,7 +31,7 @@
      * and <a href="https://en.wikipedia.org/wiki/TriX_%28syntax%29";>TriX</a>.
      * 
      * @see <a 
href="https://www.w3.org/TR/rdf11-primer/#section-graph-syntax";>RDF 1.1 
Primer</a>
    - *
    + * @see org.apache.commons.rdf.experimental.RDFParser
      */
     public enum RDFSyntax {
    --- End diff --
    
    Sorry, this comment is unrelated to the RDF change but I didn't notice this 
previously.
    
    Using enum for RDFSyntax is a bad idea unless it overrides an interface and 
the interface is used in method signatures instead of the enum. There are many 
other RDFSyntaxes, and enum (without implementing an interface) is only suited 
to cases where the full set are known a priori.


> 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