Toggling ActionBar visibility for SherlockFragmentActivity

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.

Three points:

• 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.

• 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).

• If viewPager can be marked as private, it is good practice to also mark it as private.

StackExchange Code Review Q#28809, answer score: 2