Original issue created by phwendler on 2014-09-01 at 11:52 AM
assert statements have two advantages:
1) they are short to write, and
2) they can be disabled at runtime for increased performance.
When you write "assert false" you are basically saying this code is never executed except in case of a bug, which means that point 2) is irrelevant.
Also point 1) is much less relevant, because "throw new AssertionError()" is not that much longer than "assert false" (compared to normal asserts which replace a three-line if block with a single line).
Instead, when you write "throw new AssertionError()" you gain a better dead-code analysis at compilation time, and a runtime check that turns a bug with potentially invalid data into a crash without any performance decrease.
I could not think of a situation in which you would like to fail with a crash in testing and continue running in production. If there is some error-recovery code afterwards which would allow you to proceed, then you would run this code while testing, too, and you should not use "assert false" but some other way to report this.
Thus I propose to report "assert false" and recommend to replace it with "throw new AssertionError()" (and similarly for the version with an error message).
Original issue created by phwendler on 2014-09-01 at 11:52 AM
assert statements have two advantages: 1) they are short to write, and 2) they can be disabled at runtime for increased performance.
When you write "assert false" you are basically saying this code is never executed except in case of a bug, which means that point 2) is irrelevant. Also point 1) is much less relevant, because "throw new AssertionError()" is not that much longer than "assert false" (compared to normal asserts which replace a three-line if block with a single line).
Instead, when you write "throw new AssertionError()" you gain a better dead-code analysis at compilation time, and a runtime check that turns a bug with potentially invalid data into a crash without any performance decrease.
I could not think of a situation in which you would like to fail with a crash in testing and continue running in production. If there is some error-recovery code afterwards which would allow you to proceed, then you would run this code while testing, too, and you should not use "assert false" but some other way to report this.
Thus I propose to report "assert false" and recommend to replace it with "throw new AssertionError()" (and similarly for the version with an error message).