I have no formal coding education so please let me know if something I did is considered bad coding
I am using a bottom navigation view for a train schedule application I am making. One of the options is a nearby functionality - when nearby is clicked on the bottom navigation drawer, the NearbyFragment is launched and my server gets a request via OKHttp automatically without additional user interaction
The problem is that if you switch from Nearby to another button on the bottom navigation view OR if you tapped the nearby button multiple times, the app would crash. Initially, this was because the runOnUiThread method wouldn't be able to find the UI thread since I moved on to a different fragment (and presumably the main thread no longer existed). I tried fixing this by creating a 1 second delay before the getLocation method was called in onViewCreated
// Delay before calling on location to prevent crashing when rapidly switching from Nearby Fragment
Handler handler = new Handler();
handler.postDelayed(new Runnable() {
@Override
public void run() {
if (getActivity() != null) {
getLocation();
}
}
}, 1000);
I also wrapped the above, as well as OKHttp onSuccess and onFailure methods in an if statement (getActivity != null) to check that the UI thread still existed before proceeding
@Override
public void onResponse(Call call, Response response) throws IOException {
if (response.isSuccessful()) {
final String myResponse = response.body().string();
if (getActivity() != null) {
getActivity().runOnUiThread(new Runnable() {
@Override
public void run() {
processJSON(myResponse);
}
});
}
}
}
I would still get crashes associated with the onFailure method with NullPointerExceptions and cannot find views (such as the snackbar) IF I changed from the NearbyFragment to a different fragment after 1 second of loading but before the loading had finished, so I wrapped the onFailure in a try/catch to catch all exceptions and ignore them. This feels like a hack, but my app no longer crashes/acts as expected when switching from from the NearbyFragment to another fragment via bottom navigation.
@Override
public void onFailure(Call call, IOException e) {
try {
mSwipeToRefresh.setRefreshing(false);
Log.e("Main", e.toString());
mSnackbar = Snackbar.make(getActivity().findViewById(R.id.swipe_container_nearby), "ERROR!", Snackbar.LENGTH_INDEFINITE);
snackBarCoordinator(mSnackbar);
mSnackbar.show();
} catch (Exception j) {
Log.e("Failed Get Closest Stations ", j.toString());
}
}
Is there a cleaner was to stop the crashes without the combination of three failsafes i.e., handler.postDelayed, (getActivity() != null), and an empty try/catch block?
To know whether activity or fragment is on UI.
For Activity, you can call isFinishing()
For Fragment call isAttached()
.
i.e before show loader, snack bar or any widgets wrap with if(...) {}
if(isAttached()) { snackbar.show(); }