Java bug vs illegal argument

Question

Really, I don't quite understand what is considered a bug in Java.

I've read that you should use an assert to check your code in only in private functions, but use an if statement to throw an IllegalArgumentException.

What I don't get is, why is the first case is a logic error, but the other case is "the outside user didn't provide the correct arguments"?

Let's say I have two functions:

public boolean isDigit(char c) { return c >= '0' && c <= '9'; }
public int toInt(char c) { return c - '0'; }

In all code I write, before calling toInt, I would check to make sure it's a digit first, or else it doesn't make sense!

But from what I've read, the user is allowed to make things that don't make sense; the code is supposed to check for that?

To make the code robust, I'd add an assert to make sure the params are right. These are only to check errors while debugging....

public int toInt(char c) { assert isDigit(c); return c - '0'; }

But I've read that's not how you're supposed to do it in Java? You're supposed to do:

public int toInt(char c) {
    if (!isDigit(c)) {
        throw new IllegalArgumentException("char is not a digit");
    }
    return c - '0';
}

This is really ugly for me to see, because I should already be checking that the param is valid before calling the function! Is the double check optimized somehow?

When I see the code above, it seems to me, it's as if simple code like this:

char c = '0';
if (isDigit(c)) {
    int i = toInt(c);
} else {
    doSomethingElse...;
}

becomes like this:

char c = '0';
if (isDigit(c)) {
    if(!isDigit(c)) {
        throw new IllegalArgumentException("char is not a digit");
    }
    int i = c - '0';
}
...

So is that normal, that everything is unnecessarily double checked? Is that the Java way of doing stuff?

If so, I want to know how an IllegalArgumentException would ever occur other than an error in logic, because I really don't get it. It really bugs me because to me it makes no sense someone would actually put an argument without first checking that it is valid. Thanks!

In short the question could be: Is it normal that everything is checked for consistency multiple times even though they are known to be correct, and how does it happen that an illegal argument is passed after it's been checked to make sure it's correct?

Errors calling private functions are yours and need fixing by you, before you ship. Errors calling public functions are someone else's, and need fixing by them before they ship, but `assert` is considered too drastic for that use. — user207421, Feb 21 '17 at 04:07
Ok, so all in all, it's just a conventional "personal choice" that makes it so it's done as such in Java? It's as if they could've put two kinds of asserts, private assert for your code and a public one for other people using it! — user, Feb 21 '17 at 04:16
You misrepresented `assert`. The purpose of `assert` is to document invariants in the code. It does not check for validity, it asserts validity that your code should already have checked. Next, your redundant code is the result of your writing. It doesn't have to be that way. You code what you need to to get the result you need. If you need something to be not `null`, one very good way is to make it a `final` attribute and reject `null` in the constructor. Then you can `assert attribute != null;` _instead of_ checking for it elsewhere in the class. — Lew Bloch, Feb 21 '17 at 06:27
There is a [thread](http://stackoverflow.com/q/2758224/4391450) about `assert` that can be intersting to read. Read the doc link in the answer too. Your last sample seems strange because this doesn't make sense to write `if(true){ if(false){throw ..} ...}` So your first condition already check the value — AxelH, Feb 21 '17 at 07:06

score 0 · Answer 1 · answered Feb 21 '17 at 06:38

"to me it makes no sense someone would actually put an argument without first checking that it is valid." And yet it happens all the time. That's how bugs happen. Best practices exist because they work in the real world, and API writes lock down invariants (things that simply must be so) because that's the only way to be certain things won't break. So "to me" is useless from an engineering standpoint, only "what is" matters.

To answer your question, "everything is unnecessarily double checked?", no. Only when someone does it wrong. To do it right, you do what's necessary only, and you prevent the need to double check.

The point of a runtime exception like IllegalArgumentException is to signal the programmer (who "to you" shouldn't have passed an illegal argument) that they messed up. That's the point of it.

You put it there when you need to prevent an illegal argument. Public methods are subject to abuse; you need to prevent or fix abuse so your program doesn't crash.

Because program crashes are bad.

"I've read that you should use an assert to check your code in only in private functions." Stop reading those authors. You don't use assert to check your code. You use program logic to check your inputs and conditions. (What you called an if.) You use assert to confirm invariants. For example, if you take in a char that is supposed to be a digit, of course you check that it's a digit! If you don't your program will crash, or worse, not crash but give bad results.

What else would you do?

The result of rejecting a non-digit is an invariant that your value is a digit. After you safely get past the exception throwing or early return, then you assert.

if (! isDigit(c)) { throw new IllegalArgumentException("non-digit");}
assert isDigit(c);

You know that assert can be, and you should expect that it is, turned off in production. It's really a form of active documentation.

Java bug vs illegal argument

1 Answers1