Tuesday, 1 September 2015

Naming Optional query methods

In the last article I outlined a pragmatic approach to Java 8's Optional class. In this one I'm looking at how we should name query methods that might return Optional.

Convenience method naming

Consider a requirement to produce a tree data structure, perhaps something like XML DOM. The basic data structure might look something like this:

 public XmlElement {
   private final String name;
   private final String content;
   private final Map<String, String> attributes;
   private final List<XmlElement> children;
 }

(This article uses this as an example, but many other data structures or domain objects face similar issues, so don't get too caught up in the "XML"-ness of the example, and it's inability to represent every last subtlety of XML.)

The key question for this article is about the convenience methods this class exposes. Using standard best practices, an element with no children should return an empty list, and an element with no attributes should return an empty map. Similarly, we can use the empty string, "", when there is no content.

 public XmlElement {
   public String getName() {...}
   public String getContent() {...}
   public Map<String, String> getName() {...}
   public List<XmlElement> getChildren() {...}
 }

This approach is fine, and allows us to fully query this immutable data structure. But it would be nice to have some additional convenience methods. The main one to consider is a method to return the value of an attribute by name. The naive implementation would be:

 public XmlElement {
   public String getAttribute(String name) {
     return attributes.get(name);
   }
 }

But, this method can return null which the last article recommended against. The not so naive implementation would be:

 public XmlElement {
   public Optional<String> getAttribute(String name) {
     return Optional.ofNullable(attributes.get(name));
   }
 }

This is better, as null is no longer returned and it is now clear that there is a failure case to consider. However, what if the calling code knows that the data it is asking for is expected to be there, and is going to throw an exception if it isn't?

The "standard" approach in that case is to use the method Optional.orElseThrow() on the result of the last method. And that is good advice. But, equally, if the same call to orElseThrow() follows lots of calls to the new helper method, it may be a sign that the convenience method isn't helping as much as it should! As such, perhaps it is worthy of a helper method itself:

 public XmlElement {
   public String getAttribute(String name) {
     String value = attributes.get(name);
     if (value == null) {
       throw new IllegalArgumentException(
           "Unknown attribute '" + name + "' on element '" + this.name + "'"));
     }
     return value;
   }
 }

(Note that this follows the last article's recommendation to use null, not Optional, within the methods of a class.)

So, this is better for the many use cases where it is a failure case anyway if the attribute is missing. ie. where the caller knows that the attribute is defined to be mandatory, and it is necessarily an exception if it is missing.

In reality though, some attributes are going to be mandatory and some are optional. Which brings us to the point of this article, and design approach for a case like this, and the ensuing naming problem. My suggestion is to have two methods:

 public XmlElement {

   // throw exception if no attribute
   public String getAttribute(String name) { ... }

   // return empty optional if no attribute
   public Optional<String> findAttribute(String name) { ... }
 }

In this approach, there are two methods, and the caller can choose one method if they want the exception when the name is not found, and the other method if they want to handle the missing case.

The need for this approach is relatively rare - it needs to be a method that can be used in both ways, mandatory and optional. It also needs to be an API that will be called enough for the value of the two helper methods to outweigh the extra cost. But when it does crop up, it needs a good naming convention as the two methods cannot be overloads. And that is what I'm proposing here.

  • Name the method that throws an exception when not found getXxx(String)
  • Name the method that returns an optional findXxx(String)

With this approach, both methods are available, and have reasonable names. It seems to me, that when this kind of situation arises, the mandatory case is typically most common, and thus it gets the "get" name.

There are other possible naming approaches to this "near overload". But I've settled on this one as the right balance for my APIs.

Note however, that this is not a suggestion to name all methods returning an Optional something like findXxx(String). Only use it when appropriate, such as when paired with an exception throwing version do I think this makes sense.

Summary

This article outlines a naming approach for when you need overloaded convenience methods on an API, typically for accessing a collection, such as a map or list. It proposes adding one method, getXxx(String), that throws an exception when the key cannot be found, and a second method findXxx(String), that returns an empty optional when the key is not found.

Update 2015-10-15

The original version of this article proposed the name "getXxxOptional()". I've changed that to "findXxx()" based on the comments and experimentation with both options. Its clear to me now that "findXxx()" is a way better method name than "getXxxOptional()" for a situation like this.

12 comments:

  1. I stumbled upon a definition a long time ago.

    Use getXxx if you are sure that the value is there (which means throwing an exception if it's not is fine and I would recommend this).
    Use findXxx if the value might not be there (is optional).

    I don't like getXxxOptional because with this the technical decision to use optional lacks into the caller.

    ReplyDelete
    Replies
    1. We use findXxx pattern on my team as well.

      Delete
    2. We use findXxx pattern on my team as well.

      Delete
    3. +1 we always use this convention

      Delete
    4. "decision to use optional lacks into the caller" - not really, because a return type of the method is Optional this decision in not "an implementation detail" and is exposed intentionally - irregardless of the method name.

      Delete
    5. Thanks for the comments. You were right about "find", so I've changed the article.

      Delete
  2. "use the method Optional.orElseThrow() on the result of the last method."

    This is code duplication. The getAttribute is a clone of the getAttributeOptional with a minimal insertion.

    ReplyDelete
  3. Just a little obvious correction to the article's code (check value, not name):
    if (value == null) {
    throw new IllegalArgumentException(
    "Unknown attribute '" + name + "' on element '" + this.name + "'"));
    }

    ReplyDelete
  4. Thanks for this wonderful article, I am still getting my head around effective use of Optional, this has helped me on my journey. Thanks

    ReplyDelete
  5. I usually stick with "opt" prefix for the Optional which I took from org.json library. See, for example, JSONObject:
    http://www.json.org/javadoc/org/json/JSONObject.html
    optXXX methods may return null while getXXX methods throw an exception if something is wrong. Now optXXX should return Optional, but I like such naming.

    ReplyDelete
  6. Just another convention which lends to the other side:
    To keep the standard "getter" from throwing Exception we use "locate" for the mandatory method.

    But on the other hand it's not really a "getter". Maybe it's a symptom of the java "getteritis" to call
    it "get" at all. Not to enough java 8 code seen to get a feeling I guess.

    Thanks for your articles, they make me think.

    Carsten

    ReplyDelete