56

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);
      }
   }
}
djm.im
  • 3,295
  • 4
  • 30
  • 45
ken
  • 3,745
  • 6
  • 34
  • 49
  • 11
    Note, that isn't `assert`. You should always throw an exception in such cases. By default, `assert` is switched off. – Tom Hawtin - tackline Mar 14 '10 at 02:45
  • I'm not sure what this `Assert` class is used for. I'm used to the `assert` statement: http://java.sun.com/j2se/1.4.2/docs/guide/lang/assert.html – FrustratedWithFormsDesigner Mar 14 '10 at 02:46
  • 1
    Are you asking whether having assertions is good (yes, it is)? Or whether the assert keyword, which lets you write an assertion and then turn it off for deployment, is more useful for this purpose then a "regular" if statement that throws an exception (not sure)? – Thilo Mar 14 '10 at 03:13
  • Sorry for the confusion i was more hinting to Assertions in general not necessarily the keyword – ken Mar 14 '10 at 03:33

7 Answers7

61

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

kukis
  • 4,489
  • 6
  • 27
  • 50
polygenelubricants
  • 376,812
  • 128
  • 561
  • 623
  • 8
    I'm under the impression that Assertions can be turned off. Wouldn't this undermined the entire goal of using assertions? – monksy Aug 25 '11 at 18:32
  • 3
    Not 100% sure about this, but I think it depends on the kind of application. I vaguely remember reading this in Code Complete. Let's say you have a medical device with an embedded java program. It would be running with assertions enabled so it will crash instead of returning an erroneous reading and you just need to restart and use it again. On the other hand, let's say that it is a game, and the worse that can happen is that a pixel on the screen is the wrong colour, you could disable assertions in production then. Well, at least that's my understanding. This answer was very illuminating btw. – DPM Nov 30 '11 at 20:18
  • 2
    I forgot to add: that's just for production. Typically you'll have assertions enabled during testing, so, at least in that particular scenario (testing, development) where you can fully control to have assertions enabled, you don't lose their advantages. – DPM Dec 04 '11 at 17:19
  • 2
    At my Uni, we had a course using assertions for everything (Design By Contract style). For us, the assertions were there to assure that we weren't accidently trampling our data, and causing segfaults. Once we wanted to get some real speed out of the code, and after we ran assertions on small cases, we turned off assertions/contracts. – Eagle Jul 30 '12 at 18:19
  • 1
    @Eagle - the problem is that you can never be sure you have found all of the (relevant) bugs and that it is safe to turn the assertions off. – Stephen C Sep 07 '12 at 12:51
  • 4
    @monksy - assertions implemented using `assert` can be turned off using a JVM command-line option. Assertions implemented as per the OP's example can only be turned off if the class / method is designed to allow it. – Stephen C Sep 07 '12 at 12:54
  • It is different assert and org.springframework.util.Assert util. The first can be turned off, the second not. – David García González Nov 09 '12 at 16:46
  • 1
    If you turn off assertions in the production environment your chance that the application behaviour differs is higher. – lrxw Jan 10 '14 at 10:45
  • It depends if you need performance. With a game you wouldn't want every screen draw to be checking the pixel index is valid. But it might be useful in development to have assertions checking the pixel indexes, especially if those indexes are being calculated via some complicated math. – LegendLength Jul 08 '17 at 07:21
28

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.

Andy Stabler
  • 1,309
  • 1
  • 15
  • 19
Michael Myers
  • 188,989
  • 46
  • 291
  • 292
18

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.

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.

UPDATE

In Java 7 (and later) the java.util.Objects class provides a requireNonNull convenience method to test if an argument is null and raise an exception. You use it like this:

 SomeType t = ...
 SomeType tChecked = Objects.requireNonNull(t);

or

 SomeType tChecked = Objects.requireNonNull(t, "t should be non-null");

However, note that this method raises NullPointerException rather than IllegalArgumentException.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • @djm.im - Webapp is a recognized alternative form of web app. Only correct someone's spelling if it is actually incorrect. – Stephen C May 13 '20 at 15:12
5

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
  • 502
  • 2
  • 9
2

In very large and poorly designed/maintained systems, if you're looking to improve predictability in methods that are, say, 6000 lines long and nobody in the company understands them anymore, it can be valuable to use the assert keyword to cause development environments to blow up, revealing bugs. But were you to implement those assertions in production, you might shortcircuit a patch that, though horribly conceived, fixed a problem. You want to fix that bad patch by discovering it in the dev environment, not production. So you would turn asserts on at development time, and turn them off in production.

Another valid use of the assert keyword at development time is to insert validity checks into algorithms that must execute in sub-millisecond times and are well enough insulated from unpredictable or untested callers. You may not be able to afford to preserve the validity check in production in such a case, though it's still very useful in development. On the other hand, if the source of the parameters you're validating is unpredictable or could become so (if it's determined partly by user input, for example), you can probably never afford to skip the check, even in production, and should take the performance hit as a cost of doing business. (In this last case, you probably wouldn't want to use an assert.) But you should opt for asserts to eliminate a production-time validity check only after profiling tells you you simply can't afford the overhead.

nclark
  • 1,022
  • 1
  • 11
  • 16
1

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
  • 616,129
  • 168
  • 910
  • 942
0

I'm keeping my assertions in released binaries but with modified behavior: abort is not called but stacktrace is collected.

More details here: http://blog.aplikacja.info/2011/10/assert-to-abort-or-not-to-abort-thats-the-question/

zb226
  • 9,586
  • 6
  • 49
  • 79
Dariusz Cieslak
  • 306
  • 3
  • 3
  • This approach seems interesting for production but someone has got to go and check the log file from time to time, in the mean time users may have been confronted to bugs which are symptoms of the failing assertions. If assertions aborted immediately some bugs will never occur... the sooner you fail the closer you are to the cause of the problem. – Christophe Roussy Oct 29 '12 at 13:42
  • I decided to **remove aborts from asserts** after "assert crash" case reported from customer's QA. I encourage developers to use asserts even if some of them may be wrong - developers are sure they won't break whole application by one wrong assert. – Dariusz Cieslak Apr 14 '13 at 15:10