Setting a OnClickListener in a loop

Question

I am trying to set a OnClickListener to a image in a loop. If the params platform is "android" then use market app, instead of default browswer. Is there a better solution to my exception handling or eliminatingsome of the if-else ?

private void setupListener(ImageView image, final String platform,
 final String urlLink) {
 image.setOnClickListener(new View.OnClickListener() {
 @Override
 public void onClick(View v) {
 if (platform.equalsIgnoreCase(PLATFORM_ANDROID)) {
 // open with market app
 String packageName = extractPackageName(urlLink);
 if (packageName != null) {
 try {
 Intent market = new Intent(Intent.ACTION_VIEW);
 market.setData(Uri.parse("market://details?id="
 + packageName));
 activity.startActivity(market);
 } catch (Exception e) {
 e.printStackTrace();
 startInBroswer(urlLink);
 }
 } else {
 startInBroswer(urlLink);
 }
 } else {
 // open with default broswer.
 startInBroswer(urlLink);
 }
 }
 });
}
private void startInBroswer(String urlLink) {
 Intent browser = new Intent(Intent.ACTION_VIEW, Uri.parse(urlLink));
 activity.startActivity(browser);
}

Answer 1

Here is another version with guard clauses:

private void setupListener(final ImageView image, final String platform, 
 final String urlLink) {
 image.setOnClickListener(new View.OnClickListener() {
 @Override
 public void onClick(View v) {
 if (!platform.equalsIgnoreCase(PLATFORM_ANDROID)) {
 startInBroswer();
 return;
 }
 final String packageName = extractPackageName(urlLink);
 if (packageName == null) {
 startInBroswer();
 return;
 }
 try {
 final Intent market = new Intent(Intent.ACTION_VIEW);
 market.setData(Uri.parse("market://details?id=" + packageName));
 activity.startActivity(market);
 } catch (final Exception e) {
 // TODO: log the exception
 startInBroswer();
 }
 }
 private void startInBroswer() {
 final Intent browser = new Intent(Intent.ACTION_VIEW, 
 Uri.parse(urlLink));
 activity.startActivity(browser);
 }
 });
}

Two things to notice:

1. I've removed the urlLink parameter of the startInBroswer method.

2. It is a bad idea to use printStackTrace() in Android exceptions.

Answer 2

Define field listener and assign it as OnClick handler. You'll have only one object.

View.OnClickListener mListener
@Override
public void onCreate(Bundle savedInstanceState) {
 mListener = new View.OnClickListener() {
 @Override
 public void onClick(View v) {
 if (platform.equalsIgnoreCase(PLATFORM_ANDROID)) {
 // open with market app
 String packageName = extractPackageName(urlLink);
 if (packageName != null) {
 try {
 Intent market = new Intent(Intent.ACTION_VIEW);
 market.setData(Uri.parse("market://details?id="
 + packageName));
 activity.startActivity(market);
 } catch (Exception e) {
 e.printStackTrace();
 startInBroswer(urlLink);
 }
 } else {
 startInBroswer(urlLink);
 }
 } else {
 // open with default broswer.
 startInBroswer(urlLink);
 }
 }
 }
 super.onCreate(savedInstanceState);
}
private void setupListener(ImageView image, final String platform,
 final String urlLink) {
 image.setOnClickListener(mListener);
}

• wtsang02: Nice tip. Thanks but it doesn't solve the problem with the clauses which I am more concerned about.