Search code examples
javaapi-design

As a library author, is it poor design to expect users to pass null to method parameters based on the situation?


I'm attempting to write a library which will be used by external users, and I'm stuck on some basic design decisions.

I'm writing a simple POJO class which will hold some information regarding OAuth2 token.

This class will need to meet the following conditions:

  • If the token is a permanent token, the user needs to pass only the token.
  • If the token in a temporary token, the user needs to pass the token, expirationDate, and the refreshToken.
  • There will never be a scenario where only one of expirationDate and refreshToken will be null. Either both will be null or both will be non-null.

This is what I have so far:

public TokenInformation(String token, Date expirationDate, String refreshToken) {
    Objects.requireNonNull(token, "token parameter cannot be null.");
    this.token = token;

    if ((expirationDate != null) && (refreshToken != null)) {
        this.expiresIn = expiresIn;
        this.refreshToken = refreshToken;
        isPermanentToken = false;
    } else if ((expirationDate == null) && (refreshToken == null)) {
        this.expiresIn = null;
        this.refreshToken = null;
        isPermanentToken = true;
    } else {
        // throw some exception here
    }
}  

To be honest, I'm not entirely happy with the way the code looks.

But I did have the following ideas:

  • Have two constructors. One with only one parameter (for permanent token) and the other with all three parameters (temporary token). However, my concern is that users will not read the documentation properly and will use the permanent token constructor for a temporary token.
  • Have two different factory methods instead of constructors. These factory methods will be clearly named and so the chances of a user using the wrong method will be slim. Additionally, the user will not be forced to explicitly pass in null.

I think the second idea might be the best approach. For instance, I can't think of any Java APIs which require us to pass null and that might be a hint that the code I pasted is a bad idea. Additionally, Java makes use of factory methods quite a bit, and so it's won't be an unfamiliar pattern for users of my library.

I would like other people's opinions though. So please let me know if you need any other information.


Solution

  • I would prefer to encapsulate the Permanent and Temporary token behavior into their respective domains so any user of your library clearly knows what kind of token is being instantiated

    Suggested classes in my opinion:

    /**
     * The base class encapsulates the behavior of generic token
     */
    public class AbstractToken {
    
        protected String token;
    
        // other properties that exist very closely with token
    
        public String getToken() {
            return token;
        }
    }
    

    Domain for Permanent Token

    /**
     * The domain encapsulates the behaviour of Permanent token - token that never expires
     */
    public class PermanentToken extends AbstractToken {
    
        // more attributes that makes general token as Parmament token
    
        /**
         * Instantiates a new Permanent token - this token never expires
         *
         * @param token the token
         */
        public PermanentToken(String token) {
            this.token = token;
        }
    }
    

    Domain for Temporary Token:

    /**
     * The domain for Temporary token.
     */
    public class TemporaryToken extends AbstractToken {
    
        private Date expirationDate;
        private String refreshToken;
    
        // more attributes that makes general token as temporary token
    
        /**
         * Instantiates a new Temporary token with token expiry date and refresh token
         *
         * @param token          the token
         * @param expirationDate the expiration date
         * @param refreshToken   the refresh token
         */
        public TemporaryToken(String token, Date expirationDate, String refreshToken) {
            this.token = token;
            this.expirationDate = expirationDate;
            this.refreshToken = refreshToken;
        }
    }
    

    Now the user of your library clearly knows what kind of token he/she wants to instantiate and use.

    P.S. - I guess you would be able to keep better names for your domains and you as deep into the business for your library.