tags:

views:

513

answers:

5

Is it a good practice to use Assert for function parameters to enforce their validity. I was going through the source code of Spring Framework and I noticed that they use Assert.notNull a lot. Here's an example

   public static ParsedSql parseSqlStatement(String sql) {
    Assert.notNull(sql, "SQL must not be null");}

Here's Another one

public NamedParameterJdbcTemplate(DataSource dataSource) {
            Assert.notNull(dataSource,
                    "The [dataSource] argument cannot be null.");
            this .classicJdbcTemplate = new JdbcTemplate(dataSource);
        }

        public NamedParameterJdbcTemplate(JdbcOperations classicJdbcTemplate) {
            Assert.notNull(classicJdbcTemplate,
                    "JdbcTemplate must not be null");
            this .classicJdbcTemplate = classicJdbcTemplate;
      }

FYI, The Assert.notNull (not the assert statement) is defined in a util class as follows

public abstract class Assert { public static void notNull(Object   object, String   message) {
    if (object == null) {
         throw new IllegalArgumentException  (message);
     }
 }

}

Thank you

+1  A: 

Yes it's a good idea. You're enforcing the contracting of the interface or class. If there is a contract violation you want to detect it as soon as possible. The longer you wait the more unpredictable the results can be and the harder it can be to diagnose.

When you explicitly check like this you should also provide an information message that when viewed in a log file can give useful context to help find the root cause or even just to realize you've made a wrong assumption about what the contract is.

cletus
+6  A: 

In principle, assertions are not that different from many other run-time checkings.

For example, Java bound-checks all array accesses at run-time. Does this make things a bit slower? Yes. Is it beneficial? Absolutely! As soon as out-of-bound violation occurs, an exception is thrown and the programmer is alerted to any possible bug! The behavior in other systems where array accesses are not bound-checked are A LOT MORE UNPREDICTABLE! (often with disastrous consequences!).

Assertions, whether you use library or language support, is similar in spirit. There are performance costs, but it's absolutely worth it. In fact, assertions are even more valuable because it's explicit, and it communicates higher-level concepts.

Used properly, the performance cost can be minimized and the value, both for the client (who will catch contract violations sooner rather than later) and the developers (because the contract is self-enforcing and self-documenting), is maximized.

Another way to look at it is to think of assertions as "active comments". There's no arguing that comments are useful, but they're PASSIVE; computationally they do nothing. By formulating some concepts as assertions instead of comments, they become ACTIVE. They actually must hold at run time; violations will be caught.


See also: the benefits of programming with assertions

polygenelubricants
A: 

Yes it is good practice.

In the Spring case, it is particularly important because the checks are validating property settings, etc that are typically coming from XML wiring files. In other words, they are validating the webapp's configuration. And if you ever do any serious Spring-based development, those validation checks will save you hours of debugging when you make a silly configuration mistake.

EDIT : But note that there is a BIG difference between a library class called Assert and the Java assert keyword which is used to define a Java assertion. The latter form of assertions can be turned off at application launch time, and should NOT be used for argument validation checks that you always want to happen. Clearly, the Spring designers think it would be a really bad idea to turn off webapp configuration sanity checks ... and I agree.

Stephen C
+4  A: 

Those asserts are library-supplied and are not the same as the built-in assert keyword.

There's a difference here: asserts do not run by default (they must be enabled with the -ea parameter), while the assertions provided by the Assert class cannot be disabled.

In my opinion (for what it's worth), this is as good a method as any for validating parameters. If you had used built-in assertions as the question title implies, I would have argued against it on the basis that necessary checks should not be removable. But this way is just shorthand for:

public static ParsedSql parseSqlStatement(String sql) {
    if (sql == null)
        throw new IllegalArgumentException("SQL must not be null");
    ...
}

... which is always good practice to do in public methods.

The built-in style of asserts is more useful for situations where a condition should always be true, or for private methods. The language guide introducing assertions has some good guidelines which are basically what I've just described.

Michael Myers
A: 

Based on Sun's guide on assertions, you should not use assertions for argument checking in public methods.

Argument checking is typically part of the published specifications (or contract) of a method, and these specifications must be obeyed whether assertions are enabled or disabled.

kiwicptn