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

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

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

    https://github.com/apache/commons-rdf/pull/27#discussion_r100208441
  
    --- Diff: api/src/main/java/org/apache/commons/rdf/api/RDFSyntax.java ---
    @@ -178,17 +206,62 @@ private RDFSyntax(final String name, final String 
mediaType, final String fileEx
          * The <code>fileExtension</code> is compared in lower case, therefore 
it
          * might not be equal to the {@link RDFSyntax#fileExtension} of the 
returned
          * RDFSyntax.
    +     * <p>
    +     * The list of syntaxes supported is at least those returned by
    +     * {@link #w3cSyntaxes()}.
          * 
          * @param fileExtension
          *            The fileExtension to match, starting with <code>.</code>
          * @return If {@link Optional#isPresent()}, the {@link RDFSyntax} 
which has
    -     *         a matching {@link RDFSyntax#fileExtension}, otherwise
    +     *         a matching {@link RDFSyntax#fileExtension()}, otherwise
          *         {@link Optional#empty()} indicating that no matching file
          *         extension was found.
          */
         public static Optional<RDFSyntax> byFileExtension(final String 
fileExtension) {
    -        final String ext = fileExtension.toLowerCase(Locale.ENGLISH);
    -        return Arrays.stream(RDFSyntax.values()).filter(t -> 
t.fileExtension.equals(ext)).findAny();
    +        final String ext = fileExtension.toLowerCase(Locale.ROOT);        
    +        return w3cSyntaxes().stream().filter(t -> 
t.fileExtension().equals(ext))
    +                .findAny();
    +    }
    +    
    +    /**
    +     * Return the RDFSyntax with the specified {@link #name()}.
    +     * <p>
    +     * The list of syntaxes supported is at least those returned by
    --- End diff --
    
    Changed to 
    > This method support all syntaxes returned by {@link #w3cSyntaxes()}
    
    There is no good way from a static class method to allow user extension; at 
least without having a discovery mechanism (classpath sensitive) or mutable 
setters/registrations (initialization sensitive) - I would argue it is out of 
scope for this particular method to support that, as Commons RDF only  target 
RDF 1.1.  
    
    If you want we could change these lookup method to take a variable/optional 
list of `Iterable<RDFSyntax>` as parameters? 
    
    (The parser/writer methods would with this PR support `RDFSyntax` from 
"elsewhere" - e.g. we could return supported syntaxes from each `RDF` instance)


> RDFSyntax should be interface, not enum
> ---------------------------------------
>
>                 Key: COMMONSRDF-47
>                 URL: https://issues.apache.org/jira/browse/COMMONSRDF-47
>             Project: Apache Commons RDF
>          Issue Type: Bug
>          Components: api
>    Affects Versions: 0.2.0
>            Reporter: Stian Soiland-Reyes
>            Assignee: Stian Soiland-Reyes
>
> [~p_ansell] raises in [pull request 
> 25|https://github.com/apache/incubator-commonsrdf/pull/25#discussion_r85231845]
> {quote}
> 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.
> {quote}



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to