1

I have an async operation inside of Java 8 which return an onError callback or an onSuccess callback. I need to return inside of my method if the operation was a success or not. So I am returning a boolean to state this information. The problem I am facing is I get the following compilation error:

error: local variables referenced from an inner class must be final or effectively final

Googling the error I can see you this type of operation is not allowed, but then how can I return if the operation was a success or not?

 public Boolean addUser(String email, String password) {

    Boolean isSuccess = false;

    Map<String, AttributeValue> item = new HashMap<String, AttributeValue>();
    item.put("email", new AttributeValue(email)); //email
    item.put("password", new AttributeValue(password)); //password

    dynamoDB.putItemAsync(new PutItemRequest().withTableName("Users").withItem(item), new AsyncHandler() {
        @Override
        public void onError(Exception excptn) {

        }

        @Override
        public void onSuccess(AmazonWebServiceRequest rqst, Object result) {
            isSuccess = true;
        }

    });

        return isSuccess;

}
user2924127
  • 6,034
  • 16
  • 78
  • 136
  • 8
    As I see you use AsyncHandler, so basically it means that it may be executed sometime later. I.e. at the moment when you `return` it could be not completed yet, so you don't know if it was an error or success. So you should rethink your design - the onSuccess should call something which is using the return value. – kan Jan 02 '16 at 21:16
  • The variable needs to be final because within the anonymous inner class, it cannot be referred to from the outer scope. – OneCricketeer Jan 02 '16 at 21:19

2 Answers2

8

First I will explain what the error message means, then I will tell you what is wrong with your design and suggest what to do.

First, the error message. isSuccess is a local variable, that is, as soon as the method addUser finishes, it is gone. However, the AsyncHandler instance you create might live longer than that. If it does, it would refer to a variable which doesn't exist anymore.

This would not be a problem if the variable were final or effectively final. Then we would know that the variable will never change, so we could just copy the variable to the newly created object and refer to that copy. Note that in case of a reference variable, it is the reference that doesn't change, the object it refers to might still be modified.

To circumvent this problem, the creators of Java have decided that from an anonymous inner class (or a lambda expression) you cannot refer to a local variable that is not final (or effectively final). The error message tells you that you're doing exactly that.

So now to your design. Your design cannot work in this way. The AsyncHandler instance will probably live until after the method finishes. Its methods will be probably executed in a different thread, so it's likely that its methods are called after your addUser method finishes. So what should addUser return? At the moment that it finishes, it might not even know whether the operation was successful not!

You perform an operation asynchronously, that is, on a different thread. This operation might take 1 millisecond, or 1 minute, or even 10 years. If you want to see if that operation was successful, you must somehow communicate its result to the thread that executes addUser.

Inter-thread communication is not a trivial task, but you're lucky: the putItemAsync returns a Future instance, and the Future class is intended for exactly what you want to do: find out if an asynchronous operation has finished and obtain its result.

To check whether the asynchronous operation has finished, poll the isDone() method of the returned Future. If it finished, you can obtain the result of the operation by calling the get() method of the Future. If the operation was aborted abnormally because it threw an exception, get() will throw an ExecutionException whose getCause() is the thrown exception.

Or, of course, you decide to run the operation synchronously (on the same thread) using the putItem method. Then your method will wait for the operation to finish and you have the result immediately.

Solomon Slow
  • 25,130
  • 5
  • 37
  • 57
Hoopje
  • 12,677
  • 8
  • 34
  • 50
1

This is not a good practice and also, it might not work. In your code, new AsyncHandler() will be running in a separate thread. Since it's not a blocking call, main thread will not wait for it's completion.

So, what will happen is, just after calling dynamoDB.putItemAsync(...), the return statement will get executed even before your AsyncHandler() finishes it's execution and changes the value of isSuccess. That means, return isSuccess; might return the default value false.

Note: If you really want to do it this way, you can make the boolean variable as a field variable. 

public boolean isSuccess = false;
Msp
  • 2,493
  • 2
  • 20
  • 34
  • 2
    Not "will", just "may", typical race condition, i.e. undefined behaviour – kan Jan 02 '16 at 21:26
  • Because field variable can be accessed and changed within inner class. We can't change a final variable. I know it's a bad way. But considering this case, I think it can solve the problem – Msp Jan 02 '16 at 21:40
  • *static* was a mistake. Just a field variable would do the trick – Msp Jan 02 '16 at 21:45
  • Note that even if you use the dirty field solution, there is no reason to use `Boolean` objects here, a plain `boolean` will do. – Holger Jan 04 '16 at 10:07