current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
up vote 3 down vote favorite

This works but feels clunky - any suggestions for improvement?

public static <T extends View> T findViewByClassReference(View rootView, Class<T> classReference){
  if(classReference.isInstance(rootView)){
    return classReference.cast(rootView);
  }
  if(rootView instanceof ViewGroup){
    ViewGroup viewGroup = (ViewGroup) rootView;
    for(int i = 0; i < viewGroup.getChildCount(); i++){
      View child = viewGroup.getChildAt(i);
      View match = findViewByClassReference(child, classReference);
      if(match != null){
        return classReference.cast(match);
      }
    }
  }
  return null;
}
share|improve this question
up vote 4 down vote accepted

This works but feels clunky

This seems fine. It does the job well. I don't see how this can be done simpler, and I'm not aware of other existing method in the SDK that can help simplify this.

any suggestions for improvement?

I have only minor suggestions:

  • The method uses depth first search to find a view. Depending on the actual tree of views in the application, this might be slow, for example when you have very deep trees but the class you're looking for is at a shallow level. In such case a breadth first search would perform better. This is not something to "fix", just something to keep in mind, and adjust to your actual application as necessary

  • Another point to keep in mind is that the method will return the first view with matching class. When there are multiple views of the same class, a user might not get the one expected

  • As with any utility method, it's good to add a nice javadoc. That will be a good place to mention about the previous points (search algorithm used, returns first match).

  • A common name for Class instances is clazz. I suggest to use that name to make the code look more familiar to other developers, and it's shorter too than classReference

  • The "ByClassReference" in the method name is redundant, because this is implied by the method signature

  • The formatting could be slightly more readable if you apply auto reformat in Android Studio

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.