4

In Google's article on communicating with Fragments, the author gives the following example of checking to determine if a fragment's calling activity implements a required interface:

try {
    mCallback = (OnHeadlineSelectedListener) activity;
} catch (ClassCastException e) {
    throw new ClassCastException(activity.toString()
        + " must implement OnHeadlineSelectedListener");
}

Normally, I prefer explicit checks to try/catch blocks. I this case, I would think the following example would be preferable:

if (activity instanceof OnHeadlineSelectedListener) {
    mCallback = (OnHeadlineSelectedListener) activity;
} else {
    throw new ClassCastException(activity.toString()
        + " must implement OnHeadlineSelectedListener");
}

Is there a reason to prefer one checking strategy over the other? Is there another strategy that should be used when defining interfaces to Fragments?

emerssso
  • 2,376
  • 18
  • 24
  • You could just do the bare cast, with neither an instanceof nor a try/catch, and trust that the exception that the cast would throw normally will contain the information you need. – Louis Wasserman Feb 17 '15 at 18:36
  • I agree this check doesn't seem strictly necessary, but Google recommends it, so I was curious if there was a particular advantage to their pattern. – emerssso Feb 17 '15 at 18:39
  • I personally don't use such a check because if I write good code, then that error will literally never happen. And a possible reason for the `try{}catch{}` option could be that it makes more sense to throw an exception in a catch block than in an `else`. But really, throwing an exception is the wrong way to handle errors in production anyway. – Reed Feb 17 '15 at 18:54
  • Never, but never, use exceptions as a way of controlling program flow. It is bad design and produces unmaintainable spaghetti code. – Simon Feb 17 '15 at 18:58
  • @Jakar and Simon: I agree that throwing the exception is not the best way to handle the failure (I was mimicking the example used by Google). I'm more concerned about the best way to check to see if the Fragment's interface contract is violated, so that unexpected behavior can be avoided. – emerssso Feb 17 '15 at 19:06
  • 1
    Check out [this question](http://stackoverflow.com/questions/16364185/how-to-determine-if-a-java-class-implements-a-particular-interface) and related ones. If it implements the interface, then it must implement the method. – Simon Feb 17 '15 at 19:11
  • @Simon what's the difference between isAssignableFrom() and instanceof? – emerssso Feb 17 '15 at 19:29

3 Answers3

2

Is there a reason to prefer one checking strategy over the other?

Mainly just personal preference. instanceof is more explicit for someone reading your code. try{}catch{} functionally does the same thing

Is there another strategy that should be used when defining interfaces to Fragments?

Not that I'm aware of.

Further explanation: (since I felt like rambling)

It should not be an "unexpected" problem. You should never worry about that error if you write your code well. (ignore this, in response to OP's comment)

Also worth mentioning that Google's recommended approach of throwing a new error adds unnecessary code since the default exception will contain the info you need. Furthermore, throwing the error creates a bad user experience.

The performance impact of either option will be negligible (unless you're doing something unreasonable, like looping and checking thousands of times), so (performance-wise) you can use whichever one fits your preferred coding style.

My guess is that Google recommends the try{} catch{} option because it makes more sense to throw an exception from a catch block than from an else. It probably is also because the author of that article personally prefers the try{}catch{} style.

Functionally, both methods do the same thing. The instanceof check is, however more explicit in its meaning since the code clearly states what you're checking for and if you must do a check, I'd recommend the instanceof approach.

Reed
  • 14,703
  • 8
  • 66
  • 110
  • My interest in handling this stems from Fragments that are in a library shared by multiple developers who may not have access to the Fragment's source. I want to be able to clearly communicate the cause of problems they may encounter. – emerssso Feb 17 '15 at 19:33
0

Normally, I would prefer the instanceof method as throwing and catching exceptions is thought to be expensive. Also, I think that the instance method is more readable.

antonio
  • 18,044
  • 4
  • 45
  • 61
0

Please see both of my previous answers for how I handle both:

Fragments(https://stackoverflow.com/a/27716730/950427):

public MyAdapter(Fragment fragment) {
    try {
        this.mAdapterCallback = ((AdapterCallback) fragment);
    } catch (ClassCastException e) {
        throw new ClassCastException("Fragment must implement AdapterCallback.");
    }
}

Activities(https://stackoverflow.com/a/27716788/950427):

public MyAdapter(Context context) {
    try {
        this.mAdapterCallback = ((AdapterCallback) context);
    } catch (ClassCastException e) {
        throw new ClassCastException("Activity must implement AdapterCallback.");
    }
}

Special Cases for Fragments that need Context:

private Activity activity;

public MyAdapter(Fragment fragment) {
    this.fragment = fragment.getActivity(); // use as Context
    try {
        this.mAdapterCallback = ((AdapterCallback) fragment);
    } catch (ClassCastException e) {
        throw new ClassCastException("Fragment must implement AdapterCallback.");
    }
}
Community
  • 1
  • 1
Jared Burrows
  • 54,294
  • 25
  • 151
  • 185