Search code examples
androidoptimizationonclicklistenerinstance-variables

Code optimization-Declaring views referenced by more than one method and implementing onclicklistener


I am often confused whether to create an instance views and use it in different methods OR to avoid using instance views and pass views between different methods?Is it good practice to implement onClickListener?And is it good practice to initialize views separately in different methods and avoid using instance views? Which is the better approch among below three?

1.Avoiding instance variables and NOT implemeing onClickListener

public class XYZActivity extends BaseActivity {
  @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);

        initViews();

    }
private void initViews() {
        ImageView ivScanner = (ImageView) findViewById(R.id.ivBanner);
        TextView tvName = (TextView) findViewById(R.id.tvOfferName);
        TextView tvText = (TextView) findViewById(R.id.tvOfferText);
        TextView tvDetail = (TextView) findViewById(R.id.tvOfferDetail);

menthodXYZ(ivScanner,tvName);

tvName.setOnClickListener(new View.OnClickListener() {
    @Override
    public void onClick(View view) {

    }
}); 
tvText.setOnClickListener(new View.OnClickListener() {
    @Override
    public void onClick(View view) {

    }
}); 

tvDetail.setOnClickListener(new View.OnClickListener() {
    @Override
    public void onClick(View view) {

    }
}); 
}

2.Creating instance variables and implementing OnClickListener

public class XYZActivity extends BaseActivity implements View.OnClickListener{
private ImageView ivScanner;
private TextView tvName,tvText,tvDetail;
 @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);

        initViews();

    }
private void initViews() {
        ivScanner = (ImageView) findViewById(R.id.ivBanner);
        tvName = (TextView) findViewById(R.id.tvOfferName);
        tvText = (TextView) findViewById(R.id.tvOfferText);
        tvDetail = (TextView) findViewById(R.id.tvOfferDetail);
  tvName.setOnClickListener(this); 
  tvText.setOnClickListener(this);
  tvDetail.setOnClickListener(this);
  ivScanner.setOnClickListener(this);


...
}

    @Override
    public void onClick(View view) {
        switch (view.getId())
        {
            case R.id.ivScanner:
                ...
            break;
            case R.id.tvName:
              ....
            break;
            case R.id.tvText:
              ....
            break;
            case R.id.tvDetail:
              ....
            break;
        }
}

3.If a variable is needed in different methods then initializing it separately in different methods to avoid instance variables and implementing OnClickListener

public class XYZActivity extends BaseActivity implements View.OnClickListener{

  @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);

        initViews();

    }
private void initViews() {
        ImageView ivScanner = (ImageView) findViewById(R.id.ivBanner);
        TextView tvName = (TextView) findViewById(R.id.tvOfferName);
        TextView tvText = (TextView) findViewById(R.id.tvOfferText);
        TextView tvDetail = (TextView) findViewById(R.id.tvOfferDetail);
        ...
}
private void methodXYZ()
{
           ImageView ivScanner = (ImageView) findViewById(R.id.ivBanner);
            TextView tvName = (TextView) findViewById(R.id.tvOfferName);
            TextView tvText = (TextView) findViewById(R.id.tvOfferText);
            TextView tvDetail = (TextView) findViewById(R.id.tvOfferDetail);
...
}


    @Override
    public void onClick(View view) {
      ImageView ivScanner = (ImageView) findViewById(R.id.ivBanner);
        TextView tvName = (TextView) findViewById(R.id.tvOfferName);
        TextView tvText = (TextView) findViewById(R.id.tvOfferText);
        TextView tvDetail = (TextView) findViewById(R.id.tvOfferDetail);
        switch (view.getId())
        {
            case R.id.ivScanner:
                ...
            break;
            case R.id.tvName:
              ....
            break;
            case R.id.tvText:
              ....
            break;
            case R.id.tvDetail:
              ....
            break;
        }
}

Solution

  • First of all - findByView is considered to be an expensive operation that is desirable to perform once, so your variant 3 is very expensive from this point of view.

    Secondly, if your ... in the 2nd variant means

    menthodXYZ(ivScanner,tvName);
    
    tvName.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View view) {
    
        }
    }); 
    
    tvText.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View view) {
    
        }
    }); 
    
    tvDetail.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View view) {
    
        }
    }); 
    

    --> your code with single onClick has no sense =)

    But if it means:

    menthodXYZ(ivScanner,tvName);
    tvName.setOnClickListener(this); 
    tvText.setOnClickListener(this); 
    tvDetail.setOnClickListener(this); 
    

    --> you can use it, and I think that it is the best of your variants, because you:

    • you can use all of your views instances in any method of your Activity
    • you avoiding of additional findById method calling

    Why your first variant is not good (it's only my personal opinion) - you put all of your clicks handlers logic into one single place, and if click handlers has a lot of code, it will be hard to debug it.

    ... Also, I can suggest you another variant =) You can use ButterKnife library from Jake Wharton. It will remove all your boilerplate code for creating views instances and onClickListeners:

    public class MyActivity extends BaseActivity {
    
        private Unbinder unbinder;
    
        @BindView(R.id.app_version)
        TextView appVersionTextView;
    
        @BindView(R.id.my_image_view)
        ImageView myImageView;
    
        @Override
        protected void onCreate(Bundle savedInstanceState) {
            super.onCreate(savedInstanceState);
            unbinder = ButterKnife.bind(this);
        }
    
        @Override
        protected void onDestroy() {
            super.onDestroy();
            unbinder.unbind();
        } 
    
        @OnClick(R.id.my_need_click_view)
        public void clickToMySuperView() {
            Log.i("TAG", "click to my super view!");
        }
    
        @OnClick(R.id.my_need_click_view_2)
        public void clickToMySuperView() {
            Log.i("TAG", "click to my super view 2!");
        }
    
        ...
    
    }
    

    UPDATE

    Look on the following peace of code:

    public class MyActivity extends BaseActivity implements View.OnClickListener {
    
        protected void onCreate(Bundle savedInstanceState) {
            super.onCreate(savedInstanceState);
            initView();
        }
    
        private void initView() {
            ImageView myImageView = (ImageView) findViewById(R.id.my_image_view);
            TextView myTextView = (TextView) findViewById(R.id.my_text_view);
    
            myImageView.setOnClickListener(this);
            myTextView.setOnClickListener(this);
        }
    
        @Override
        public void onClick(View view) {
            switch (view.getId()) {
                case R.id.my_image_view:
                   // special code for image view
                   break;
    
                case R.id.my_text_view:
                    // special code for text view.
                    break;
    
                default:
                    // no actions.
                    break;
            }
        }
    
    }
    

    In this peace of code we initialize our views instances only once, then set click listener to them. onCreate method, according to Activity lifecycle, calling only once (if Activity creates in the first time, or recreates it's state after destroying). In OnClick method our views not initializing, we just need id from View parameter.

    Then, look on the following peace of code:

    public class MyActivity extends BaseActivity implements View.OnClickListener {
    
        private ImageView myImageView;
        private TextView myTextView;
    
        protected void onCreate(Bundle savedInstanceState) {
            super.onCreate(savedInstanceState);
            initView();
        }
    
        private void initView() {
            myImageView = (ImageView) findViewById(R.id.my_image_view);
            myTextView = (TextView) findViewById(R.id.my_text_view);
    
            myImageView.setOnClickListener(this);
            myTextView.setOnClickListener(this);
        }
    
        @Override
        public void onClick(View view) {
            switch (view.getId()) {
                case R.id.my_image_view:
                   // special code for image view
                   break;
    
                case R.id.my_text_view:
                    // special code for text view.
                    break;
    
                default:
                    // no actions.
                    break;
            }
        }
    
    }
    

    In this case we initialize our views instances and saves them into our Activity object. But, again, we initialize them only once - in onCreate method. And again, in onClick method we don't search this views again through findById method.

    Hope, it helps.