Toggling ActionBar visibility for SherlockFragmentActivity

Question

Method implemented in parent class (extends SherlockFragmentActivity) (source)

ViewPager viewPager; // initialized in onCreate method
public void toggleActionBar() {
 try {
 FrameLayout.LayoutParams lp = new FrameLayout.LayoutParams(
 FrameLayout.LayoutParams.MATCH_PARENT,
 FrameLayout.LayoutParams.WRAP_CONTENT);
 if (getSupportActionBar().isShowing()) {
 getSupportActionBar().hide();
 lp.topMargin = 0;
 } else {
 getSupportActionBar().show();
 lp.topMargin = getSupportActionBar().getHeight();
 }
 viewPager.setLayoutParams(lp);
 } catch (Throwable t) {
 Log.e(TAG, "Toggle of Actionbar failed", t);
 }
}

Call made from one of Fragments in ViewPager (source)

try {
 ((DilbertFragmentActivity) getSherlockActivity()).toggleActionBar();
} catch (Throwable t) {
 Log.e(TAG, "Toggle of Actionbar failed", t);
}

MinSdkVersion is equal 8, ActionBar support is done by ActionBarSherlock library.

• palacsint: Is it a bad idea to use printStackTrace() in Android Exceptions? at Stack Overflow

Answer 1

Three points:

1. getSupportActionBar() is called four times within the same method. Store it as a local variable and use the variable four times instead, to improve code cleaniness and readability.
2. When catching exceptions, be as specific as possible. Catching Throwable is a horrible idea. Only catch the exceptions you need to catch (Which involves neither Errors or RuntimeExceptions. You shouldn't even wrap ((DilbertFragmentActivity) getSherlockActivity()).toggleActionBar(); inside a try-catch statement. If you think that something can go wrong (like a NullPointerException), make sure that it doesn't go wrong before you try to do it (by checking if something is not null for example).
3. If viewPager can be marked as private, it is good practice to also mark it as private.