Which Java string value is preferred to return: blank or null?

Question

I came across this snippet and am wondering if I should return blank in this case.

Which is more practical to return: blank or null?

public String getInfo() {
    String result = "";
    String uri = clientConfiguration.getUri();
    String regex = "http://(.*)/consolidate";
    Pattern pattern = Pattern.compile(regex);
    Matcher matcher = pattern.matcher(uri);
    if (matcher.find()) {
        result = matcher.group(1);
    }
    return result;
}

Edit: Should I change it to:

 if(matcher.find()) { 
   return matcher.group(1); 
 } 
 return null;

Answer 1

The general question you ask "Which is better, "" or null?" is off-topic for CodeReview, but your code snippet has a number of reviewable items....

Regex Usage

Compiled Pattern performance

  String regex = "http://(.*)/consolidate";
  Pattern pattern = Pattern.compile(regex);
  Matcher matcher = pattern.matcher(uri);
  if(matcher.find()){
      result = matcher.group(1);
  }

This snippet makes it look like you know what you are doing, with the Pattern compile, etc. But, there is no performance benefit in the way you have done this. Compiled Java Pattern instances are thread-safe, and compiling them for one-time-use is not useful. It is common practice to make the Pattern a static-final field:

private static final Pattern MYPATTERN = Pattern.compile("http://(.*)/consolidate");

Then you can re-use that compiled pattern as much as you like, in any method, in any thread, like:

  Matcher matcher = MYPATTERN.matcher(uri);
  if(matcher.find()){
      result = matcher.group(1);
  }

find vs. matches

Now, matcher.find() and matcher.matches() are different methods.

• find() will scan the input looking for any point inside the input where the pattern will match....
• matches() does just one scan, and it matches the entire input string against the entire pattern.

With the input junkhttp://a/consolidate/junk:

• find() will find your pattern
• matches() will not find your pattern

The pattern

Now, as for the actual Regex... it appears that you want your pattern to match the HTTP 'host' against which you have the 'consolidate' path.... but, your pattern will match a lot of things which I would consider to be unexpected... for example, your pattern will return the following:

http://myhost/consolidate   from  http://http://myhost/consolidate/consolidate
myhost/consolidate          from  http://myhost/consolidate/consolidate
myhost:8080                 from  http://myhost:8080/consolidate
                            from  http:///consolidate

Each of the above input values will produce unexpected results.

Correct Regex

There is not a correct regex for matching URL's.... even if a regex appears that it will match, it is still not the correct solution ;-)

Solution

Use the java.net.URI class to validate your input. I have an example here, where I choose to return null if there is no configured URI, or throw an exception if there is a configured URI and that URI is not a valid value. This would be 'sensible' for many configurations, I expect.

public String getInfo() {
      String urival = clientConfiguration.getUri();
      if (urival == null || urival.isEmpty()) {
          return null;
      }
      try {
          URI uri = new URI(urival);
          return uri.getHost();
      } catch (URISyntaxException e) {
          throw new IllegalStateException("The configured URI is not valid: " + e.getMessage(), e);
      }
}

Answer 2

I agree with fge, it depends on the context. A third approach: it might be better to throw an exception in that case.

Another note: you don't need to declare the result variable at the beginning of the method, declare on the first use. Actually, you could completely get rid of it:

    if (matcher.find()) {
        return matcher.group(1);
    }
    return "";

(See also: Effective Java, Second Edition, Item 45: Minimize the scope of local variables It has a good overview on the topic.)

Answer 3

As "getInfo" does not hint at something functional or even crucial - only displaying the info, its usage should not fail with a NullPointerException or need to catch a thrown not-found exception. So neither return null here, nor throw an IllegalStateException, but return an empty string.

Better however would be to return an informative string "(Info missing)".

Should the result of getInfo() being used instead of only displayed, throw an exception. In this way the usage can be skipped. This already might be the case, if you embed the result is a surrounding text message. A comparable strategy is done for the hideous NumberFormatException.

Returning null, like in JDBC for field values, requires discipline by the user, and makes only sense in a context where more such "optional" data can be retrieved (= the null does not come unsuspected).

For good order: In Java 8 there also is the class Optional to indicate the optional presence of an object, and getting the object in a next step. (Overkill here)

Answer 4

Really, does every return value need to be assigned to a value? The code would be far clearer if you removed these spurious variables. result should be removed as well; just use two return statements. And for such a small piece of code, I would also suggest shortening matcher, both to avoid confusion with Matcher, and to allow further statements to be more concise.

/** 
 * You need to add javadoc!
 * @return The info, or an empty string if there is no info.
 */
public String getInfo() {
    Matcher m = Pattern.compile("http://(.*)/consolidate")
            .matcher(clientConfiguration.getUri());

    return m.find()?m.group(1):"";
}

Some people insist one must use a full if statement, regardless of the occasion. In any case, don't use a result variable if returns will do, like this:

if(m.find()) return m.group(1);
return "";

As for returning "" or null, as long as you document it in your javadoc (with an @return), either way is fine.

Answer 5

If your question is whether you could refactor the code to return null instead of "", without knowing more about your code, I would say no.

Changing the return value from "" to null may break existing code calling your method, and expects it to always return a string value.

Answer 6

It is always better to return null, in place of blank if you are not getting any info. Your method name goes to getInfo(). Returning blank means you get some information but it is blank. But returning null means you did not get any information at all.

It is all upon you to decide what exactly you want to deduce from the returned value.

return null;