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
I bet that you’ve come up with a
boolean[]
as work around substituting a simpleboolean
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-elementboolean
arrayb
. 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 implementCallable
interface (which contrary toRunnable
is capable of producing the return value) to achieve the same result.That’s how such implementation might look like:
In order to run the
Callable
in a separate thread, you can use an instance ofExcecutorService
, 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 bothRunnable
andCallable
).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 byExecutors
class.Here’s a simplistic example using with a single-threaded executor:
You can also perform a check
isDone()
to find out whether theFuture
instance contains the result: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 thatboolean
asvolatile
. 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 theRunnable
, 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 likeConcurrentHashMap
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