Learn, Share, Build

254
September 25, 2017, at 10:40 AM

In the below posted code, I am tring to code a utility method and I want to check if an object is null or not and a string is null and not empty. so, I coded the way shown below with throwing some exception, but I think the code could have been coded in a better way because the way I coded it has nested try and catch blocks.and i do not think it is a good style

please guide me to better code the belwo method

code:

public static boolean isFragmentShowing(Activity activity, String tag) throws Exception {
    try {
        if (activity != null) {
            FragmentManager fragmentManager = FragmentUtils.getFragmentManagerInstance(activity);
            try {
                if (tag != null && !tag.equals("")) {
                    Fragment fragment = fragmentManager.findFragmentByTag(tag);
                    return (fragment != null && fragment.isVisible())? true : false;
                } else {
                    throw new NullPointerException("isFragmentShowing: fragment tag passed is null or empty");
                }
            } catch (NullPointerException e) {
                Log.e(TAG, "Error: " + e.getMessage().toString());
                System.exit(1);
                return false;
            }
        } else {
            throw new NullPointerException("isFragmentShowing: context reference is null");
        }
    } catch (NullPointerException e) {
        Log.e(TAG, "Error: " + e.getMessage().toString());
        System.exit(1);
        return false;
    }
}
Answer 1

There are two part of your application. One is request validation and another one is application logic. Separate request validation and application logic. It will be easier to read and maintains. Here is my try in bellow

public static boolean isFragmentShowing(Activity activity, String tag) throws Exception {
//validate request 
if(activity == null) {
    // throw exception or return value 
}   
if (tag == null && tag.equals("")){
    // throw exception or return value 
}
// rest of the part
FragmentManager fragmentManager = FragmentUtils.getFragmentManagerInstance(activity);
Fragment fragment = fragmentManager.findFragmentByTag(tag);
return (fragment != null && fragment.isVisible())? true : false;
}
Answer 2

If all you're going to do with the exception is

Log.e(TAG, "Error: " + e.getMessage().toString());

then you don't need the exception, you just need a string. As I said on your previous question, catching NullPointerException is rarely the correct thing to do; more generally, using exceptions for control flow is a pretty dubious practice. And using System.exit is rarely what you really want to do.

You can create a method something like:

boolean logMessageAndExit(String message) {
  Log.e(TAG, "Error: " + message);
  System.exit(1);
  return false;
}

and then call in your code like this:

if (activity == null) {
  return logMessageAndExit("isFragmentShowing: context reference is null");
}
if (tag != null && !tag.equals("")) {
  return logMessageAndExit("isFragmentShowing: fragment tag passed is null or empty");
}
Fragment fragment = fragmentManager.findFragmentByTag(tag);
return fragment != null && fragment.isVisible();

Returning a boolean here is a mere convenience to allow you to return it: this convinces the compiler that execution never goes past that line, even though the return is never really executed.

(You could make it return something Throwable instead, so you can throw logMessageAndExit, to make it more clear that it is abnormal).

Answer 3

Alright, here's how your isFragmentShowing method should be like. You see I've removed ALL of the try/catches. This is because your method already throws a checked Exception and the code calling your static method would need to wrap the call to isFragmentShowing inside the try/catch none the less. You can catch the NPE there quite easily and even print out its stack trace which would let you know which instance was essentially null. Either fragment or activity.
The only time we need to actually throw an NPE is when tag.equals("") returns true (since that won't throw an exception).
I've also replaced the last ternary operator return by just fragment != null && fragment.isVisible() since it means the same thing (true is returned if the expression evaluates to true and false on the right is returned otherwise, why not the return the result of the expression itself?)

And here's the code:

public static boolean isFragmentShowing(Activity activity, String tag) throws Exception {
    FragmentManager fragmentManager = FragmentUtils.getFragmentManagerInstance(activity);
    if (tag.equals("")) {
        throw new NullPointerException("isFragmentShowing: fragment tag passed is empty");
    }
    Fragment fragment = fragmentManager.findFragmentByTag(tag);
    return fragment != null && fragment.isVisible();
}
Rent Charter Buses Company
READ ALSO
Learn, Share, Build

Learn, Share, Build

I would like to use firebase to store general info on the author, submission date, and file system location (on the server)Then when the user opens the app, it will bring them to where I would curate specific photos (think IFunny, etc)

249
Learn, Share, Build

Learn, Share, Build

I would like to know if or how it is possible to contribute into a map of a parent component

243
Learn, Share, Build

Learn, Share, Build

I want to create chat ,sent and accept friend request like Facebook in androidCan any one help

230
Learn, Share, Build

Learn, Share, Build

enter image description hereI have rooted my phone and also have super user access

215