0

I'm trying to learn Threads and Tasks for days now ... but still can't implement it in my app ... help plz.

I want to run all Database operations in a background thread other than the application thread. I have a class that manages the Database queries ... in this class i surrounded the executeQuery statement with a task:

public class Database {

ResultSet rs;

public ResultSet execQuery(PreparedStatement stmnt) throws SQLException {       
    Task<ResultSet> task = new Task<ResultSet>() {
     @Override protected ResultSet call() throws Exception {
             if (isCancelled()) {
             }                    

                 ResultSet execRs = stmnt.executeQuery();
                 return execRs;
         }
     };     
    task.run();
    task.setOnSucceeded(new EventHandler<WorkerStateEvent>(){
        @Override
        public void handle(WorkerStateEvent event) {                
             rs = task.getValue();
        }            
    });
    return rs;
}
// remaining Code
}

As you can see the method should return a ResultSet but when i call this from another place it raise a null pointer ... the result set returned by this method is null.

So what did i do wrong here?

Update #1 @James_D Thank you for this great link ... i think i finally understand the concept ... but still have a small problem with implementing it ... for example in my authentication method after the user is authenticated i want to check if that user has an open shift ... so following your link i changed the method to this:

    private boolean isShiftOpen(int userId, int branchId, int comId) throws SQLException, ClassNotFoundException {
//        final boolean success = false; 
        Task<Shift> task = new Task<Shift>(){
            @Override
            protected Shift call() throws Exception {
                return ShiftDAO.getShift(userId, branchId, comId);
            }            
        };
        task.setOnFailed(e -> {
            System.out.println("isShiftOpenTask Faild!!");
            success = false;
        });
        task.setOnSucceeded(e -> {
            System.out.println("isShiftOpenTask Succeeded!!");
            Shift shift1 = task.getValue();
            System.out.println("User Open Shift Exists ... returning true");
            SessionBean.setShiftId(shift1.getShiftId());
            SessionBean.setUserId(shift1.getUserId());
            SessionBean.setUserBranch(branchId);
            success = true;
        });
        exec.execute(task);
        return success;

    }

I have two problems: 1- The exec.execute(task) raise a nullpoint exception. 2- I wanted to use a boolean variable returned by this method ... the only way i could access such a variable is to define it outside the method ... but then if i want to use another method like this one i must declare another boolean variable for it to ... does this sound right to you?

Thank you for your time Gado

Gado
  • 23
  • 1
  • 7
  • Problem 1. is unrelated; you can easily debug the NPE, just see https://stackoverflow.com/questions/218384/what-is-a-nullpointerexception-and-how-do-i-fix-it. For 2., again, if you are running this on a background thread, the `isShiftOpen` method will return *before* the task is complete. That's basically the whole point of using a thread. So you cannot return a value that depends on what happens in the task. What are you going to do with that value? Whatever it is, that should go into the `onSucceeded` and `onFailed` handlers. – James_D Oct 02 '17 at 16:42

1 Answers1

2

You call

task.run();

which executes the task on the current thread, i.e. that statement will not complete until the task finishes. Then you call

task.setOnSucceeded(...);

which essentially says "when the task succeeds, set the instance variable rs to the result of the task. However, by the time you call this, the task has already succeeded (or possibly failed), so there is no way for the handler to be invoked.

You could fix the null result by reversing the order of these calls, i.e. do

public ResultSet execQuery(PreparedStatement stmnt) throws SQLException {       
    Task<ResultSet> task = new Task<ResultSet>() {
     @Override protected ResultSet call() throws Exception {
             if (isCancelled()) {
             }                    

                 ResultSet execRs = stmnt.executeQuery();
                 return execRs;
         }
     };     
    task.setOnSucceeded(new EventHandler<WorkerStateEvent>(){
        @Override
        public void handle(WorkerStateEvent event) {                
             rs = task.getValue();
        }            
    });
    task.run();
    return rs;
}

However, since you are executing the task on the current thread, it's not really clear what the point of using a task at all is: you may as well just execute the database query directly in your execQuery method and return the result directly. In other words, the above code is equivalent to

public ResultSet execQuery(PreparedStatement stmnt) throws SQLException {       

     rs = stmnt.executeQuery();
     return rs;

}
James_D
  • 177,111
  • 13
  • 247
  • 290
  • Thanks for the response ... First i tried your suggestion to call the run() after the setOnSucceeded() but still returns null. Second my intention is not to run this on the same thread i want to run it on a separate thread.(Question updated) – Gado Oct 02 '17 at 14:39
  • @Gado You should make sure it is not throwing an exception. E.g. do `task.setOnFailed(event -> task.getException().printStackTrace());` (before you call `run()`, obviously). If you want to do this in a background thread, however, it will be *impossible* to return the result set from the `execQuery()` method, because that method will likely exit *before* the database query is complete. You probably need to rethink the design completely here, if you want to run the query in a background thread. – James_D Oct 02 '17 at 14:50
  • @Gado Have a look at https://stackoverflow.com/questions/30249493/using-threads-to-make-database-requests – James_D Oct 02 '17 at 14:58
  • @Gado I rejected the edit... it should just go away silently now. – James_D Oct 02 '17 at 16:42