AdvancedExpressionFolding icon indicating copy to clipboard operation
AdvancedExpressionFolding copied to clipboard

Confused folding when check if a variable is null while it has a exception type

Open rofinily opened this issue 7 years ago • 5 comments

The original code:

Throwable exception = null;
if (exception != null) {
    throw exception;
}

The processed code:

assert exception == null;

Isn't it confused when reading?

rofinily avatar Nov 02 '18 07:11 rofinily

What do you see wrong with this? What would make it more readable?

ciscorucinski avatar Nov 06 '18 17:11 ciscorucinski

assert exception == null means I assert this exp is true, or an exception would be thrown;

// it means I want to throw this exception if it is not null(or I will get an unexpected NPE)
if (exception != null) {
    throw exception;
}

Can you understand the difference between these two statements?

I think this kind of code needn't be processed, just let it be.

rofinily avatar Nov 07 '18 08:11 rofinily

Those two expressions are the same. Just because you can express it in English differently, doesn't mean it is different

assert exception == null

This means, throw an exception if the condition is false

if (exception != null) {
    throw exception;
}

This means, throw an exception if the condition is true

So these two statements are the exact same in the same sense that [ 4 - 2 ] = [ 4 + ( -2 ) ]. They are the same.

ciscorucinski avatar Nov 07 '18 17:11 ciscorucinski

// equals to assert exception == null
if (exception != null) {
    throw new NullPointerException();
}

// the only difference is throw the checked variable itself
if (exception != null) {
    throw exception;
}

rofinily avatar Nov 09 '18 01:11 rofinily

OK, now you brought up a valid difference. The point of this plugin is to make the code more readable.

You can basically refactor the original code into the assert statement. The assert statement is more simple to read and understand. then the if statement version. Simple and concise is important. It tries to keep the folded statement's meaning as close to the original as possible.

I would argue that many people would agree with the current implementation as being more than enough; however, if you absolutely need that difference to be shown at all time, then you can disable the "assert" fold in the settings.

Is that difference really that important that you need to have it shown at all times? I am sure that other aspects of your code is far more important since a simple mouse-hover can reveal the original code

ciscorucinski avatar Nov 09 '18 16:11 ciscorucinski