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.
CodePudding user response:
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 behavior of your code is consistent.
Also, HashMap
is not meant to be used in multithreaded environment.
CodePudding user response:
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