Is this overuse of Java?
I am trying to get around using a keyword assert
in Java. As I understand it, the right case is to test what should always be true.
I am worried that I am overusing statements, however.
Here's an example:
private BodyParams() {
assert revokedDoc != null : "revokedDoc must not be null";
assert revokedDoc.getStatus() == DocumentStatus.Revoked : "document is not revoked";
assert !isBlank(revokedDoc.getDocType()) : "docType should not be blank";
assert revokedDoc.getIssuedDate() != null : "doc should have issue date";
assert revokedDoc.getSendingOrg() != null
&& !isBlank(revokedDoc.getSendingOrg().getName())
: "sending ord should exists and name should ne populated";
if (registeredUser) {
assert revokedDoc.getOwner() != null
&& !isBlank(revokedDoc.getOwner().getFirstName())
: "owner should exists and first name should be populated";
this.ownerFirstName = revokedDoc.getOwner().getFirstName();
this.docUrl = Application.PUBLIC_HOSTNAME
+ controllers.routes.DocumentActions.viewDocument(
revokedDoc.getId()
).url();
} else {
this.ownerFirstName = null;
this.docUrl = null;
}
if (revokedDoc.getStatus() == DocumentStatus.Available) {
assert !isBlank(revokedDoc.getFriendlyName())
: "friendly name should not be blank for picked-up docs";
this.friendlyName = revokedDoc.getFriendlyName();
} else {
this.friendlyName = null;
}
this.docType = revokedDoc.getDocType();
this.issueDate = revokedDoc.getIssuedDate();
this.issuerName = revokedDoc.getSendingOrg().getName();
}
This example assumes that the revokedDoc field came from the database and the correct validation was performed when it was inserted. These statements support this assumption. Is this overkill?
edit: I should mention that this is for development code only. Approvals will not be included in production. I use assertions to ensure that data to be known, good data from a reliable source in production, behaves in development
source to share
It doesn't look right. For simplicity's sake, there are two broad categories of problems that can arise and require variable validation:
- Your method is receiving or using an argument that may not be what you expect, and your method should have the appropriate argument validation and throw
IllegalArgumentException
orNullPointerException
or whatever as needed. Example: client code passed in a null argument and you have no control over that code. - Your method uses some of the internals of the class, and you should have appropriate unit tests to ensure that these internals are always consistent and that your methods can use them without additional checks.
In your case, the method that creates the object revokeDoc
needs to ensure that it is in the correct state after creation and take appropriate action otherwise, such as throwing an exception and discarding any changes. So your method BodyParams
can just use the object without all the assertions that clutter your code at the wrong time: if revokeDoc
incompatible, it might be too late to do something about it and should have been discovered earlier.
Related post: Vs Assertion Exception
source to share
Assert
really useful for execution, which should always be true inside a library or module . It intends to check invariants in your code (control flow, internal, etc.) and it is a bad idea to use it to ensure that your code is used correctly (you have exceptions for this).
As a consequence, your public interface should never be assertive-based: when you have a public method and want to validate an input parameter, it's usually best to throw IllegalArgumentException
.
There is good documentation on assertions here.
In your example, I think you should be using exceptions instead of assertions. It is not a bad idea to perform some validation on the data coming from the database (even if it was checked on input), but assertion can be disabled in production code and you need to think about how you should handle such malformed content.
source to share
This can be a stubborn question. However, I would take the following solutions:
- This method is exposed to the outside world (via an interface, JAR file, user input field, or wherever you can get the raw data from a source that is not under your control) - then I should have a valid actual validation that will result in exception.
- Am I relying on assertion to execute the code correctly? If so, I shouldn't. Assertions must be disabled during runtime.
- Is this statement always true? and if yes, I'm going to use it off-case for just debugging - then yes, use assertion instead of code comment. When things go wrong, include statements and figure out what happened.
source to share
You need to consider two scenarios: development code and production code .
Since the Java operator is assert
disabled by default (and only adds a small overhead by checking the global static flag, which is resolved by passing it -ea
to the VM), I wouldn't consider this overhead, as it helps you spot problems early in development ( assumes you have included assertions in your development environment).
On the other hand, you say, "... A valid check was performed when it was inserted ..." - so how do you know that the value has not been changed in the database in the meantime? If security is important to your system (I'm just assuming it is), one basic pattern is that you shouldn't trust anything you get from the outside. Means check the values you read from the database, but in this case is assert
not a suitable tool. Use normal check and exception code for this.
source to share
The best practice, according to the OO method, is to check the parameters you get. And create regular checks for others. If in your case you should get something like this:
private BodyParams(revokedDoc)
[...]
asserts of the params
if(isBlank(revokedDoc.....)
All assets look good and this is a way to make sure the method has everything . But they should be helping out what's going wrong, not making your program work.
source to share