18

Here is my code snippet:

Someclass someObject;
boolean success = true;
try {
    someObject = someOperation();
} catch (Exception e) {
    success = false;
}
if (success) {
    int number = Integer.valueOf(someObject.someMethord());
}

and within the line:

 int number = Integer.valueOf(someObject.someMethord());

the Java compiler throws and error saying

Error: variable someObject might not have been initialized`.

However, if success equals true, then there is no way someObject will not have been initialized, why am I getting this error?

Sled
  • 18,541
  • 27
  • 119
  • 168
user2916610
  • 765
  • 1
  • 5
  • 12
  • 2
    What value should have `someObject` in case of exception ? This is the "way" the compiler see this would not be initialized, but the value of `success` will be defined at runtime, so it can't be used to see that this would not be a problem. – AxelH Dec 01 '17 at 09:05
  • @DevSolar I tried to retract the vote but didn't notice that, first the retraction didn't worked, and that the comment stays... – AxelH Dec 01 '17 at 12:57
  • 1
    The compiler does only a very trivial static analysis and that's by design. [It keeps the compiler fast and and the user sane](https://stackoverflow.com/a/47594217/581205). – maaartinus Dec 01 '17 at 13:14
  • 16
    "Don't set a flag; set the data." http://wiki.c2.com/?EwDijkstraQuotes – Pete Kirkham Dec 01 '17 at 13:26
  • 3
    I want to elaborate on @Pete’s comment: regardless of compiler diagnostics, writing such code simply isn’t a good idea I’m afraid. It’s hard to tell from your simplified example but as much as possible you should straighten out your code paths and simplify dependencies. In this simple case you could completely remove the `success` variable and put all the code in the `try` block. In your actual code you can probably perform something equivalent. Or don’t `catch` at all inside the current method. – Konrad Rudolph Dec 01 '17 at 13:46
  • What if the method you called returns null? Then you wouldn't have an exception, but the variable would be initialized anyway – Délisson Junio Dec 01 '17 at 14:07
  • @DélissonJunio well, technically he would have an exception, because someMethord() would be executed on a null object :)... – Mixxiphoid Dec 01 '17 at 14:14
  • @Mixxiphoid Yup! Isn't that exactly what the compiler is warning about? Look at the error description: it shows exactly on the line calling someMethod, as in, the variable can be null when executing that line – Délisson Junio Dec 01 '17 at 14:16
  • @DélissonJunio no it is not. NotInitialized != null. If an object is null it IS initialized. Frankly the compiler doesn't care if you want to execute something on null, that is only a warning, not an error. The compiler is complaining because there is potentially no object at all. – Mixxiphoid Dec 01 '17 at 14:29
  • 13
    This code is *horrible* and you should not use this pattern. *You eat all exceptions regardless of the exception type, and those exceptions might be handled by your caller*. Only eat exceptions that you understand. The right thing to do is to simply write `S s = someOperation(); if (s != null) { ... }` – Eric Lippert Dec 01 '17 at 16:00
  • Technically, `someOperation()` is executed before its result is assigned to `someObject`. So, if `someOperation()` raises an exception error, the assignment does not take place, and the variable `someObject` is *really* left uninitialised. – rexkogitans Dec 01 '17 at 20:56
  • 1
    @EricLippert: You don't even need the null check here. If `someOperation` succeeds, `s` has a usable value; otherwise, you never get to the point of using it anyway due to the exception. – cHao Dec 01 '17 at 22:08

7 Answers7

51

The compiler doesn't analyze the relation between your success flag and the initialization of the someObject variable.

As far as the compiler is concerned, someObject may not be initialized if an exception occurs.

You can solve that issue by setting the variable to null in your catch block (and instead of checking your success variable, check that someObject != null).

Or you can move the int number = Integer.valueOf(someObject.someMethord()); statement inside your try block.

Eran
  • 387,369
  • 54
  • 702
  • 768
  • 1
    Oh man, that is nice answer. You are right, compiler provides just let me say "static" analysis of the code, so from the scope of the compiler is variable just declared, but not properly inicialized. – xxxvodnikxxx Dec 01 '17 at 09:10
  • 11
    @xxxvodnikxxx Static analysis *could* tell that `someObject` is always fully initialised when used. That’s why *we* (the readers) can tell. However, this kind of static analysis is computationally hard in the general case (although it’s fairly trivial in this small example) and is therefore not implemented by Sun’s Java compiler. – Konrad Rudolph Dec 01 '17 at 13:27
  • 13
    @KonradRudolph more accurately: the Java Language Specification mandates exactly which checks the compiler should implement. Unlike C, the Java compiler is not given a wide latitude to reason about things like code correctness. – nneonneo Dec 01 '17 at 17:30
11

The Java Language Specification (JLS) defines exactly how the compiler should analyze code like this.

In your case, the local variable someObject is not definitely assigned before it is used in the if block. Definite assignment, covered in chapter 16 of the JLS, defines the exact rules by which a variable can be considered assigned to ("initialized").

It analyzes the try and if statements separately. After the try, someObject is not definitely assigned, because it isn't assigned in the catch block. In the if, the condition could be true or false. If it were true, you get an error because someObject is not definitely assigned at this point.

The Java compiler is not permitted to analyze this code and "figure out" that success can only be true iff someObject is assigned, because the language rules prescribe the precise analysis that must be performed. This isn't a case of the compiler being insufficiently smart - this is a case of the Java Language Standard being strict.

Note that if you use if(false) instead of if(success) you won't get an error, because the JLS specifies that false is a constant expression, and thus that the body of the loop will never execute.


The flag variable, in any case, is unnecessary. Moving the dependent code into the try, or setting the variable to null in the declaration and explicitly checking for someObject != null are all approaches that are simpler to understand and much less error-prone.

nneonneo
  • 171,345
  • 36
  • 312
  • 383
10

You can change your declaration like this:

Someclass someObject = null;

Or you can do everything into the try-catch to be sure that someObject will be initialized properly

try {
    Someclass someObject = someOperation();
    int number = Integer.valueOf(someObject.someMethod());
} catch (Exception e) {
    //...
}
hnefatl
  • 5,860
  • 2
  • 27
  • 49
Guillaume Barré
  • 4,168
  • 2
  • 27
  • 50
3

The compiler does only a very trivial static analysis and that's by design. Static analysis can get smarter over time and you surely don't want the code to stop compiling when you switch to an older compiler version.

Another reason is keeping the compiler fast. Smart analysis is important for optimizations, but its' costly. Optimizations are not the job of javac (they happen at runtime), so javac doesn't bother.

It doesn't even recognize trivial cases like

int f(boolean b) {
    if (b) {
        return 1;
    } else if (!b) {
        return 0;
    }
}

The rules are specified in the JLS Chapter Definite Assignment.

maaartinus
  • 44,714
  • 32
  • 161
  • 320
  • Didn't understand your example... could you please develop on what the compiler could recognize in the example above? – jjmontes Dec 01 '17 at 15:27
  • 1
    @jjmontes The compiler may issue a warning that all the paths don't return a value (the else of the last if), but in reality b can only be true or false, so the "else" path will never occur. – Julien N Dec 01 '17 at 15:45
  • 3
    @JulienN Actually, the compiler *must* issue an *error*. We know that `f` always returns a result as the `else if` is equivalent to plain `else`, the compiler could know it, too, but it's obliged not to. – maaartinus Dec 01 '17 at 15:55
2

You can view this scenario as -

What happens is your try blocks fails to initialize the someObject.

This means there might be some exception in someOperation() method. Exception will be caught but someObject will not be initialized.

You can fix this by setting someObject to null or new SomeObject() in catch block.

hnefatl
  • 5,860
  • 2
  • 27
  • 49
2

What you're describing in Java is very similar in C# (there the message would be CS0165 Use of unassigned local variable 'someObject'). The solution in both languages is to make a null-assignment:

      SomeClass someObject = null;

and the warning will go away. As the other answers were describing, the reason is simply that the compiler isn't smart enough, so those warnings are kind of a trade-off.

Complete Java example:

class SomeClass
{
    public int someMethod()
    {
      return 1;
    }
}

public class JavaFiddle
{

    public static SomeClass someOperation()
    {
        SomeClass result = new SomeClass();
        return result;
    }

    public static void main(String[] args)
    {
      SomeClass someObject = null;
        boolean success = true;
        try {
            someObject = someOperation();
        } catch (Exception e) {
         success = false;
        }
        if (success) {
            int number = Integer.valueOf(someObject.someMethod());
        }
    }

}

Paste this code in a JavaFiddle
(press Ctrl and left-click to open a tab with JavaFiddle)

> Compiling...
> Running...

As you can see, in the code above the issue is already fixed, so if you paste it into the JavaFiddle tool, it will run as expected without error.

To provoke the error, change the code as follows:

SomeClass someObject; // = null;

and run it again. You will get the message:

> Compiling...

/str/JavaFiddle.java:29: error: variable someObject might not have been initialized
int number = Integer.valueOf(someObject.someMethod());
                                               ^
1 error


Note that I just took the example you provided "as is" - you should consider the hints given regarding exception handling (see comment to your question from Eric Lippert). Swallowing exceptions without really handling them usually isn't good - better deal with the cases you know in the code and let the caller handle any exceptions that might occur you don't know about (and, of course implement some logging).

Matt
  • 25,467
  • 18
  • 120
  • 187
0

the compiler has a limited understanding of what that variable might be, for example:

 public void test() {
    int y = 2;
    Integer x;
    if (y == 2) {
        x = 2;
    }

    System.out.println(x);// this will fail
}

But make that final y = 2 and the compiler will see that the only possible value of y is 2 in this case.

Eugene
  • 117,005
  • 15
  • 201
  • 306