skip to Main Content

Hey all I am trying to add to a list of map<String, String> using the code below:

@NonNull
public static List<Map<String, String>> mysqlSelect(@NonNull String select, String where) {
    Map<String, String> info = new HashMap<String,String>();
    List<Map<String, String>> list = new ArrayList<>();
    int selectCnt = select.replaceAll("[^,]","").length();
    final boolean[] b = {true};

    new Thread(new Runnable() {
        @Override
        public void run(){
            try (Connection connection = DriverManager.getConnection(URL, USER, PASSWORD)) {
                String sql = "SELECT fb_firstname, fb_lastname, fb_id FROM fbuser";
                PreparedStatement statement = connection.prepareStatement(sql);
                ResultSet resultSet = statement.executeQuery();

                while (resultSet.next()) {
                    info.put("name", resultSet.getString("fb_firstname"));
                    info.put("address", resultSet.getString("fb_lastname"));
                    info.put("phone_number", resultSet.getString("fb_id"));

                    list.add(info);
                }

                b[0] = false;
            } catch (Exception e) {
                Log.e("InfoAsyncTask", "Error reading school information", e);
                b[0] = false;
            }
        }
    }).start();

    while (b[0]) { }

    return list; //result.get("name")
}

As it loops in the while () {.. part it populates the list with the needed 3 string values. However, as it does loop over and over again it seems to populate all the previous values to what was last read instead of having new data each time?

Example:

1st loop:

name: bob barker
address: something
phone-number = 4235421212

2nd loop:

name: steve jobs
address: something again
phone-number = 8546521111

at this point the 1st loop data turns into

1st loop:

name: steve jobs
address: something again
phone-number = 8546521111

2nd loop:

name: steve jobs
address: something again
phone-number = 8546521111

So, what am I missing here?

UPDATE 1

Originally I have the following:

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_main);

    new InfoAsyncTask().execute();
}

@SuppressLint("StaticFieldLeak")
public class InfoAsyncTask extends AsyncTask<Void, Void, Map<String, String>> {
    @Override
    protected Map<String, String> doInBackground(Void... voids) {
        Map<String, String> info = new HashMap<>();

        try (Connection connection = DriverManager.getConnection(URL, USER, PASSWORD)) {
            String sql = "SELECT fb_firstname, fb_lastname, fb_id FROM fbuser LIMIT 1";
            PreparedStatement statement = connection.prepareStatement(sql);
            ResultSet resultSet = statement.executeQuery();
            if (resultSet.next()) {
                info.put("name", resultSet.getString("fb_firstname"));
                info.put("address", resultSet.getString("fb_lastname"));
                info.put("phone_number", resultSet.getString("fb_id"));
            }
        } catch (Exception e) {
            Log.e("InfoAsyncTask", "Error reading school information", e);
        }

        return info;
    }

    @Override
    protected void onPostExecute(Map<String, String> result) {
        if (!result.isEmpty()) {
            TextView textViewName = findViewById(R.id.textViewName);
            TextView textViewAddress = findViewById(R.id.textViewAddress);
            TextView textViewPhoneNumber = findViewById(R.id.textViewPhone);

            textViewName.setText(result.get("name"));
            textViewAddress.setText(result.get("address"));
            textViewPhoneNumber.setText(result.get("phone_number"));
        }
    }
}

But I wanted to put it into its own class file and call it from the onCreate.

2

Answers


  1. I bet that you’ve come up with a boolean[] as work around substituting a simple boolean flag, which you can’t change from the lambda expression since it should be effectively final. This trick would allow updating the flag, but since it might be cached by a new thread, another thread might not be able to observe that it has been updated.

    To establish a so-called happens-before relationship, you need to use AtomicBoolean instead of a single-element boolean array b. That would guarantee that the behavior of your code is consistent.

    Also, HashMap is not meant to be used in multithreaded environment.

    Addressing the updated question

    If I’ve understood the issue correctly, you’re trying to reimplement the code which was developed using deprecated AsyncTask class.

    Since previously you had a code which asynchronously generating a Map, and returning the whole thing, you implement Callable interface (which contrary to Runnable is capable of producing the return value) to achieve the same result.

    That’s how such implementation might look like:

    public class InfoTask implements Callable<Map<String, String>> {
        
        private String url;      // remove the field
        private String user;     // if you want to choose another way of providing these data
        private String password; // other than passing them to constructor
        
        // constructor
        
        @Override
        public Map<String, String> call() throws Exception {
    
            Map<String, String> info = new HashMap<>();
    
            try (Connection connection = DriverManager.getConnection(url, user, password)) {
                String sql = "SELECT fb_firstname, fb_lastname, fb_id FROM fbuser LIMIT 1";
                PreparedStatement statement = connection.prepareStatement(sql);
                ResultSet resultSet = statement.executeQuery();
                if (resultSet.next()) {
                    info.put("name", resultSet.getString("fb_firstname"));
                    info.put("address", resultSet.getString("fb_lastname"));
                    info.put("phone_number", resultSet.getString("fb_id"));
                }
            } catch (Exception e) {
                Log.e("InfoAsyncTask", "Error reading school information", e);
            }
    
            return info;
        }
    }
    

    In order to run the Callable in a separate thread, you can use an instance of ExcecutorService, which is basically a thread-management mechanism which maintains a pool of threads or a single threads and arranges execution of provided task (you can submit both Runnable and Callable).

    When the task is submitted, the executor returns you an instance of Future. You may think of it as if it’s a container wrapping the result that is expected sometime late in the future.

    To obtain an ExcecutorService you can use one of the static factory methods provided by Executors class.

    Here’s a simplistic example using with a single-threaded executor:

    InfoTask task = new InfoTask(url, user, password);
            
    ExecutorService executor = Executors.newSingleThreadExecutor();
    Future<Map<String, String>> future = executor.submit(task);
            
    // do some stuff here
            
    // NOTE: get() call is BLOCKING
    Map<String, String> resultt = future.get(5, TimeUnit.SECONDS); // don't use parameterless get(), always provide a timeout
            
    executor.shutdown(); // always invoke shutdown when executor is no longer needed
    

    You can also perform a check isDone() to find out whether the Future instance contains the result:

    if (future.isDone()) {
        Map<String, String> resultt = future.get(5, TimeUnit.SECONDS);
        // do something
    }
    
    Login or Signup to reply.
  2. You’ve got a concurrency issue – when you start using threads and updating values that a different thread can see, there’s no guarantee that what one thread sees is what the other one does, unless you use some form of synchronisation.

    (Basically, to make computers run real fast, they take some liberties about the order they do things in, and where they write down their working. It’s like having the current data updated on a whiteboard in an office, except to work quicker people are sometimes just writing things down on a notepad at their desk, and they’ll go update the whiteboard later)

    There are a bunch of approaches to dealing with this – you can get an overview at the section on concurrency in the Java documentation, and there are lots of tools at your disposal. I’d really recommend reading it (or something like Java Concurrency in Practice) because it’s a really important concept to get your head around – a lot of weird and bad stuff can happen if you don’t know what you’re doing with threading!

    But in this case, if you have a boolean that only one thread is writing to, and everything else is reading from it, you can mark that boolean as volatile. That means it updates everywhere (so everything can see the current value) as soon as it’s written. That should fix your "why is it still true" problem. (Although you might need to add it as a field on the Runnable, and store a reference to that, since you’re doing all this inside a function)

    The actual data you’re writing in the map is the bigger issue – you need to make sure that’s synchronised. You can do that by gating all access to the map (on all threads) through a synchronized block, but Java also provides a few concurrency-minded collections classes like ConcurrentHashMap that take care of that for you. But that doesn’t solve all the problems related to concurrency, like race conditions – again, you have to understand the problems around this stuff, so you can be mindful about avoiding them

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search