[jira] [Comment Edited] (OAK-553) Add NameMapper#getValidOakName that never returns null

["Thomas Mueller (JIRA)" <ji...@apache.org>]

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

Thomas Mueller edited comment on OAK-553 at 1/15/13 3:33 PM:
-------------------------------------------------------------

A relatively common pattern seems to be:

mapper.getJcrName(mapper.getOakName(name));

In fact, the above pattern is used that many times (in the nodetype package) that I wonder why there is no method that does both?

In addition to the code duplication, I'm afraid this pattern will throw a NPE and swallow the (invalid) name value, so that you don't really see the illegal name in the error message and stack trace. All you see in the log file and error message that *something* was illegal, but there is no way to find out what exactly. To simplify support, the exception should contain the illegal argument value as well.

I wonder where exactly it is really useful that getOakName returns null on an illegal name, and not throws an IllegalArgumentException (with the illegal value) directly? If the caller method forgot to handle the IllegalArgumentException, then at least we get the argument value in the exception, and not simply a NPE (without the argument) as we do now, if the caller doesn't handle the null return value correctly. 

In other words, where is it useful that the method returns null and doesn't throw an IllegalArgumentException?
                
      was (Author: tmueller):
    A relatively common pattern seems to be:

mapper.getJcrName(mapper.getOakName(name));

In fact, the above pattern is used that many times (in the nodetype package) that I wonder why there is no method that does both?

In addition to the code duplication, I'm afraid this pattern will throw a NPE and swallow the (invalid) name value, so that you don't really see the illegal name in the error message and stack trace. All you see in the log file and error message that *something* was illegal, but there is no way to find out what exactly. To simplify support, the exception should contain the illegal argument value as well.

I wonder where exactly it is really useful that getOakName returns null on an illegal name, and not throws an IllegalArgumentException (with the illegal value) directly? If the caller method forgot to handle the IllegalArgumentException, then at least we get the argument value in the exception, and not simply a NPE (without the argument) as we do now, if the caller doesn't handle the null return value correctly.
                  
> Add NameMapper#getValidOakName that never returns null
> ------------------------------------------------------
>
>                 Key: OAK-553
>                 URL: https://issues.apache.org/jira/browse/OAK-553
>             Project: Jackrabbit Oak
>          Issue Type: Improvement
>          Components: core
>            Reporter: angela
>
> we keep adding methods that convert a jcrName into an oak-name
> and throw an exception if the value returned by NameMapper#getOakName is
> null. instead of duplicating code i would like to add a second flavor
> of the method that throws if the the jcrName cannot be resolved to an oak
> name.
> i even suspect that most usages of the getOakName call that don't throw
> in case of null-value are susceptible to NPE. but we can still review those
> and possible replace them with the save method call.
> proposed patch for the interface:
> {code}
> /**
>      * Returns the Oak name for the specified JCR name. In contrast to
>      * {@link #getOakName(String)} this method will throw a {@code NamespaceException}
>      * if the JCR name is invalid and cannot be resolved.
>      *
>      * @param jcrName The JCR name to be converted.
>      * @return A valid Oak name.
>      * @throws NamespaceException If the JCR name cannot be resolved.
>      */
>     @Nonnull
>     String getValidOakName(@Nonnull String jcrName) throws NamespaceException;
> {code}

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira