Assertion statements: Remove or retain in production code

Question

I'm working in a large company and am doing my first Java project here. We have certain code style guidelines. In the Java guideline, it is said that "assertions are only intended for debugging and bug hunting, but should be removed in production code".

I personally like to write assertions to get notified of invalid program states during testing and let them be deactivated in production. But my colleagues tell me, I should remove all assert statements from the code base. When I ask about the reasons, I'm referred to the above-mentioned code style guideline for Java.

Is there any best practice that states something like "do not retain assert statements in production code"? And what is the background of such a rule?

Answer 1

First, let me say I am not advocating against the usage of asserts in languages like Java or C# in general. However, since the old days of C programming, where assertions made a lot more sense, the cases where an assert is really the most proper tool have become rare. Note in old-times C programming there was usually no in-built array bounds checking, no NULL pointer checking and no exception mechanism in the language. Moreover, performance was more frequently a real issue than it is today on modern hardware.

As you surely know, assertions are usually activated only in your debugging (byte) code, but deactivated in production (byte) code. Hence, their usage is clearly restricted to check conditions in your code indicating a bug, and they must not be used as a replacement for checking the correctness of some external input data.

However:

• What if the bug you want to prevent with a certain assertion shows up only in production?

Often, the only reasonable action in such a situation would be to stop the program in production as well, with a clear log message and stack trace where the error occurred. An assertion would sweep the problem under the rug in production and would make it likely that the error will manifest itself later at a place where it gets a lot harder to find its root cause.

Hence, whenever you feel the need to use an assertion, think twice if adding a regular test and throwing an exception (for example, an IllegalArgumentException or an IllegalStateException) would not be more appropriate.

Now you may ask:

• But what about performance?

Indeed, the fact assertions are deactivated in production code gives them a performance benefit over tests which stay activated. That often facilitates their usage — a programmer can use them a lot, most probably without investing too much thought if there will be a notable performance hit or not in the final program. Unfortunately, using a lot of assertions bears the risk of:

• Code bloat — when more than the half of the code of a function consists of assertions, the function becomes less readable.

• Introducing code which behaves differently in development than in production.

• Abusing assertions for cases where even in production throwing an exception would be more appropriate.

Thus the recommendation in your code style guideline is not the worst:

• Debug your code using assertions, if you think that helps you.

• But before your next commit (or pull request), go through all of those assertions and think twice:

  • Is the performance benefit really worth it not to use an endurable check? (If you did not measure it, the answer is most often "no")
  • What will happen when the bug you want to prevent occurs in production as well?
  • Is there a risk of making the program behave differently in production?
  • Does the additional code increase or decrease readability?

If you have gone through that checklist, you may still come to the conclusion for specific cases that an assertion is exactly what you need there — but those cases will probably be rare, and you will have to discuss it with your colleagues if the assertion should stay in the code, despite of your coding style guide.

Ideally, you can convince your colleagues to change the style guide a bit, like:

• "Assertions should be used mainly for debugging, and left in production code only after going through this checklist ... (insert the points from above here)"

Large companies often don't like such phrases in style guides, because those are guides for programmers who

• Are self-disciplined enough to take the checklist serious, and
• Know the context well enough to take production-behaviour of the whole system into account, and to take the necessary performance considerations into account.

A braindead rule like "Don't use assertions in production code" leaves a lot less room for interpretation and can be applied with a lot less thought by every newbie programmer. Actually, it can be automated. Whether that really leads to better software in the end is quite debatable.

Answer 2

Joshua Bloch has the following guide lines in Effective Java: Use normal Exceptions to check parameters in public methods. He only recommends to use assert for checking parameters for private methods.

So I would encourage you to look at your code and ask yourself if some of the assertions should be checks with regular exceptions.

Answer 3

~ OBSERVATIONS ~

One common convention is the use of a debug flag. The workflow is more or less what you described as your personal workflow, but with an extra detail added.

The addition is that there will be some sort of global variable (I use the term loosely here to mean some value that can be set or changed at runtime) the value of which controls their activation.

The implementation conventions differ from one language and / or environment to the next, but the concepts are usually the same. One value, which represents debug mode, will enable (whether by way of a script of some sort or by any other means) not only assert statements, but also various other custom tools they have created to facilitate efficient debugging.

Some implementation details that Ive seen which stuck out to me include:

• in C, an old convention was to set a DEBUG macro that was included in the compile options for debug builds. I am not sure if this is still a convention, though, as C++ developers generally consider Macros bad.

• Linux developers of command line tools usually include a --debug option that serves the same purpose.

• Various frameworks and standalone solutions now exist with the sole purpose of providing an interface to build flags at runtime.

~ SOLUTION ~

As for possible solutions, I believe that my workflow for debugging java may circumvent your issue.

My stance is that everyone working on a project will most likely have their own brand of debugging that works well for them. For example, you like to use assertstatements, and I never do. (I assume that you are talking about the keyword defined in the Java API and not the various testing frameworks' versions.) What I am guessing you use assert for, I do mostly using breakpoints and watchers pointed at any properties.

If everyone were to include their preferred helpers, this difference in debugging styles would yield some messy source code. One section would follow convention A , while the next section would be filled with B. C may or may not overlap both sections.

To avoid this without having to rewrite my reusable debug code, I will usually write it into a separate file. This may be a java class that extends a target class, or it may not be java at all. It depends on the situation.

The best way to describe this method would be to say:

Externalize your own debug code in the most efficient and convenient means for the situation at your disposal.

I usually keep my own debug script, but you could just as well share it with others. If the file is destined to stay small and will not be in the persisted source, neatness and style consistency do not matter as much.

One more solution would be to use a git workflow that allows you to "keep them without keeping them."

~ AFTERTHOUGHT ~

git rebase allows you to add only certain commits from your branch to another. If you are careful with commits, ensuring that all of your debug logic is contained within their own commit(s), you would be able to merge only the "good" code into the team's working branch. However, this would be hard to maintain across merges, so it would be a no-go without something else added to automate synchronization.That said, I am sure that a GitHub search would provide many such options, so it may be worth looking into for some users.

Answer 4

Start with the question what is it that you want to achieve.

The standard C and C++ assert is quite primitive: in a debug build it evaluates an expression and crashes the app if the expression is false. In a release build it does nothing. So there is no reason to remove it from the source code.

I never felt this was adequate. So I have a macro Assert() which performs a breakpoint if the assertion is false and the app runs under a debugger. So that gets the developers attention. It stays there forever, just as you keep unit tests forever. You don’t remove them if they passed for the last year. So this helps a lot during development and can save me if there are unexpected changes later.

Next macros Asserted() and AssertionFailed() which assert but at the same time report whether the assert was true or false. So if a condition should always be true I can still check it and handle a failing assertion. That must stay in the release code.

And there is a function FatalError with a macro AssertFatal: Used when a situation is found where it’s better to crash the app right now then to continue. That’s especially important for a released version. Of course using this isn’t done lightly.

Answer 5

to me it would seem to be an absurd practice to remove the assertion statements. the -ea and -da flag is there for a reason.

This is also related to Secure Coding practices, input must be validated, with is usually done with explicit checks in the public methods, and if the correct and private method will never see invalid input, but the assertion is to check that for example.