0

The problem is about the "int amount". I'm trying to create a system where every couple seconds an amount is added to the start amount.

public static void main(String[] args) {

    final int WIDTH = 600;
    final int HEIGHT = 600;
    int amount = 0;

    Timer timer = new Timer();
    timer.schedule(new TimerTask() {

        @Override
        public void run() {
            int amount = amount + 1;
        }
    }, 2*1000, 2*1000);

    JFrame frame = new JFrame("TimerTest");
    frame.setVisible(true);
    frame.setSize(WIDTH, HEIGHT);
    frame.setResizable(false);
    frame.setLayout(null);
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);

    JLabel label = new JLabel("");
    label.setText(String.valueOf(amount));
    frame.add(label);
  }
}
GhostCat
  • 137,827
  • 25
  • 176
  • 248
ThijsP
  • 19
  • 3
  • 1
    Note that incrementing the amount variable doesn't cause the UI to repaint, you have to call `label.setText(String.valueOf(amount))`within the ´TimerTask´ – Dorian Gray Aug 14 '18 at 19:07
  • 1
    Oddly, though, `int amount = amount = 1;` would be valid. – Andy Turner Aug 14 '18 at 19:13
  • There are many problems in your code. What are you *trying* to accomplish here? Because even if you increment `amount` in timer it will not cause `label` to also update its content via `label.setText(String.valueOf(amount));` (this instruction will be called only once in main thread and will use *current* value of `amount` at time of execution). – Pshemo Aug 14 '18 at 19:21

1 Answers1

7

Here:

int amount = amount + 1;

You are shadowing the outer defintion of amount.

Meaning: that inner class doesn't use the outer field amount. It has a local variable, and that one gets assigned to itself. But of course, the initial state of amount is unknown.

Probably you meant:

amount = amount + 1;

or simply

amount++; 

instead. Which .. will not work, in your case.

The problem is that amount is also a local variable, not a field. To make this work, you have to "promote" amount and make it a field of the enclosing class. See here for more details on why that is.

As suggested by user Arnaud: when you don't want to use a field, you should change the type of the int amount local variable to AtomicInteger, and then use its getAndIncrement() method.

GhostCat
  • 137,827
  • 25
  • 176
  • 248
  • I've tried that before but then I get an other error: "Local variable amount in an enclosing scope must be final or effectively final" – ThijsP Aug 14 '18 at 19:10
  • There are imho more satisfying solutions than creating a field. It seems to me that defining `amount` as an `AtomicInteger` is better as it does not pollute the name space. – Arnaud Denoyelle Aug 14 '18 at 19:14
  • @GhostCat I did not write that += was better. I was thinking about the problem of the local variable. This one can be solved with an `AtomicInteger`. The trick is that it is an object, not a primitive value. Define it like this : `final AtomicInteger amount = new AtomicInteger()` then update it in the inner class with `amount.incrementAndGet()`. – Arnaud Denoyelle Aug 14 '18 at 19:19
  • @GhostCat It is declared `final` so it is accessible from the inner class. But as it is an object, you don't modify the pointer when calling `amount.incrementAndGet()`, you just modify the pointed value so it does not violate the `final` keyword. – Arnaud Denoyelle Aug 14 '18 at 19:20
  • I updated the answer accordingly. Please note: the *final* is not required any more, the compiler is clever enough to figure that the local variable is **effectively final**. – GhostCat Aug 14 '18 at 19:21
  • Alternatively we can move `int amount = 0` from being local variable, and make it `TimerTask` field. This would also require moving `label.setText(String.valueOf(amount));` to `run` method but something tells me that this is what OP should do anyway if he wants to let label show current value for `amount`. – Pshemo Aug 14 '18 at 19:33