I need to make variable be used outside of the if statement. It says I need to make if final, but If I try using it it won't work

Question

I have this code:

public void onPlayerInteract(PlayerInteractEvent event) {
    final Action action = event.getAction();
    Location l1 = null;
    Location l2 = null;
    if (action == Action.LEFT_CLICK_BLOCK){
        l1 = event.getClickedBlock().getLocation();
    } else if (action == Action.RIGHT_CLICK_BLOCK) {
        l2 = event.getClickedBlock().getLocation();
    }

    Thread t = new Thread() {
        @Override
        public void run() {
            while(true) {
                try {
                    Thread.sleep(1000*60*60);
                    Location maxx = l1.getX();
                    Location maxy = l1.getY();
                    Location maxz = l1.getZ();

                    Location minx = l2.getX();
                    Location miny = l2.getY();
                    Location minz = l2.getZ();

                    if(l1.getX() > l2.getX()){
                        //I can't execute this, errors!
                    }
                } catch (InterruptedException ie) {
                }
            }
        }
    };
    t.start();

It gives me errors, and says to change l1 and l2 to finals. If I change l1 and l2 to finals, it gives me another error, where it says l1 = etc., it says to remove the final.

If a variable is final, you can't re-assign it. You need to re-work your design. — jahroy, May 25 '12 at 22:40
You have another problem: either l1 or l2 (or both!) is going to be null, but you're not checking for null before dereferencing. — jwismar, May 25 '12 at 22:42
Another problem is that he's trying to use SO as a text book. Just read about Threads, Runnables, classes, variables, etc... then re-implement your solution. — jahroy, May 25 '12 at 22:46

JB Nizet · Accepted Answer · 2012-05-25T22:51:28.723

l1 and l2 are local variables of the method onPlayerInteract(). In this method, you create an anonymous inner class which uses these local variables l1 and l2. This is only possible if l1 and l2 are final. But by definition, a final variable can only be assigned once, and you assign null, and then another value to them. So you need to make a copy of l1 and l2 to final variables, and use those final copies inside your anonymous class:

public void onPlayerInteract(PlayerInteractEvent event) {
    final Action action = event.getAction();
    Location l1 = null;
    Location l2 = null;
    if (action == Action.LEFT_CLICK_BLOCK){
        l1 = event.getClickedBlock().getLocation();
    } else if (action == Action.RIGHT_CLICK_BLOCK) {
        l2 = event.getClickedBlock().getLocation();
    }

    final Location l1Final = l1;
    final Location l2Final = l2;

    Thread t = new Thread() {
        @Override
        public void run() {
            while(true) {
                try {
                    Thread.sleep(1000*60*60);
                    Location maxx = l1Final.getX();
                    Location maxy = l1Final.getY();
                    Location maxz = l1Final.getZ();

                    Location minx = l2Final.getX();
                    Location miny = l2Final.getY();
                    Location minz = l2Final.getZ();


                    if(l1Final.getX() > l2Final.getX()){
                        // ...
                    }
                } catch (InterruptedException ie) {
                }
            }
        }
    };
    ...
}

score 1 · Answer 2 · answered May 25 '12 at 22:43

1

public void onPlayerInteract(PlayerInteractEvent event) {
    final Action action = event.getAction();
    final Location blockLocation = event.getClickedBlock().getLocation();
    final Location l1 = (action == Action.LEFT_CLICK_BLOCK) ? blockLocation : null;
    final Location l2 = (action == Action.RIGHT_CLICK_BLOCK) ? blockLocation : null;

    Thread t = new Thread() {
        ...
    }
}

answered May 25 '12 at 22:43

Kkkev

4,716
5
27
43

It doesn't work, I only want to assign l1 if it is a left click, and l2 if right click. – Thor Correia May 25 '12 at 22:46
@PlazmotechBinary: did you try this answer? It doesn't work in your code because you're trying to assign twice to a final variable, which is forbidden. This answer assigns only once, and thus fixes the problem. Read the error messages of the compiler. Thay are meaningful. – JB Nizet May 25 '12 at 22:48
It doesn't work, I only want to assign l1 if it is a left click, and l2 if right click edit - oops duplicated :P – Thor Correia May 25 '12 at 22:50
1

@PlazmotechBinary don't be so arrogant, as long as yxou undrstand nothing, please! Try it out, it works. It doesn't work the way you did it. – Ingo May 25 '12 at 22:50
@PlazmotechBinary: this is what this code does. Do you at least understand what the above code does? – JB Nizet May 25 '12 at 22:53
@ingo im sorry, but how can I do this then? I dont want any interaction, only LMB and RMB. And I want them to be assigned to different variables. If you can make it work I would congratulate and thank you. – Thor Correia May 25 '12 at 22:54
@PlazmotechBinary: do you read the comments? The above code works, and does what you want. – JB Nizet May 25 '12 at 22:56
@PlazmotechBinary Jon Skeet has a great article about this, contrasting C# closures and Java closures. Java can only do a closure/capture on the value, not on the variable itself. Basically, Java wanted you to burn the value(via final) permanently before you can use the ~~variable~~ value from outside of anonymous class. Read it at: http://csharpindepth.com/Articles/Chapter5/Closures.aspx search the word **captured** – Michael Buen May 25 '12 at 22:58

score 1 · Answer 3 · edited May 23 '17 at 11:49

You need to use final for the anonymous inner class. And as you may know, a final reference can't be modified.

The answer of JB Nizet is correct.

But notice that instead of JB Nizet code:

Location l1 = null;
Location l2 = null;
if (action == Action.LEFT_CLICK_BLOCK){
    l1 = event.getClickedBlock().getLocation();
} else if (action == Action.RIGHT_CLICK_BLOCK) {
    l2 = event.getClickedBlock().getLocation();
}

final Location l1Final = l1;
final Location l2Final = l2;

You can use the following code

final Location l1;
final Location l2;
if (action == Action.LEFT_CLICK_BLOCK){
    l1 = event.getClickedBlock().getLocation();
    l2 = null;
} else if (action == Action.RIGHT_CLICK_BLOCK) {
    l1 = null;
    l2 = event.getClickedBlock().getLocation();
} else {
    l1 = null;
    l2 = null;
}

As a local variable is never initialized (even to null), the compiler often tells you to initialize it before using it. But if you initialize it in all cases of your if/elseif/else structure, the compiler knows for sure you have initialized it in any case.

Anyway, your code doesn't seem to make any sens because in any case, l1 or l2 will be null. Thus your thread will always throw a NullPointerException.

Here's an explaination why you must use final for anonymous inner classes: Why do we use final keyword with anonymous inner classes?

Also notice that you can't use the statement if(l1.getX() > l2.getX()){ Because getX() and getY() returns Location and can't be compared using > operator. You should consider using Comparable on Location class and then do if ( l1.getX().compareTo(l2.getX()) > 0 ) { ... }

score 0 · Answer 4 · answered May 25 '12 at 22:50

0

The code inside the run is effectively its own method. You could make it a separate runnable method and pass y1 & y2 to it on instantiation, or, declare y1 & y2 within the run.

answered May 25 '12 at 22:50

Phil Freihofner

7,645
1
20
41

score 0 · Answer 5 · answered May 25 '12 at 23:06

The solution is as explained by @JB Nizet but you don't need to copy the variables IF they have setter methods. Your code would then look as follows (also fixing the null pointer as explained by @:

public void onPlayerInteract(PlayerInteractEvent event) {
final Action action = event.getAction();
final Location l1 = new Location(); // Assumming Location has a default constructor.
final Location l2 = new Location();
if (action == Action.LEFT_CLICK_BLOCK){
    l1.setX(event.getClickedBlock().getLocation().getX());
    l1.setY(event.getClickedBlock().getLocation().getY());
} else if (action == Action.RIGHT_CLICK_BLOCK) {
    l2.setX(event.getClickedBlock().getLocation().getX());
    l2.setY(event.getClickedBlock().getLocation().getY());
}

Thread t = new Thread() {
    @Override
    public void run() {
        while(true) {
            try {
                Thread.sleep(1000*60*60);
                Location maxx = l1Final.getX();
                Location maxy = l1Final.getY();
                Location maxz = l1Final.getZ();

                Location minx = l2Final.getX();
                Location miny = l2Final.getY();
                Location minz = l2Final.getZ();


                if(l1.getX() > l2.getX()){
                    // ...
                }
            } catch (InterruptedException ie) {
            }
        }
    }
};
...

}

@Kkkev asnwer should work fine also but watch out for the null pointer.

I need to make variable be used outside of the if statement. It says I need to make if final, but If I try using it it won't work

5 Answers5