HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Is this too much work for 1 listener?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thismuchtoolistenerforwork

Problem

Is this too much work for 1 touch listener? Is this bad for devices with slow CPU's?

@Override
     public boolean onTouch(View v, MotionEvent event) {
        int id = v.getId();
        switch(id){
                      case R.id.sound1:
                          if(event.getAction()==android.view.MotionEvent.ACTION_DOWN){

                          if(intLoop ==0){
                              mp1=MediaPlayer.create(MainClass.this, R.raw.item1);
                              mp1.start();
                              v.setPressed(true);
                              return true;
                          }
                          if(intLoop == 1){
                              mp1=MediaPlayer.create(MainClass.this, R.raw.item1);
                              mp1.start();
                              mp1.setLooping(true);
                              v.setPressed(true);
                              return true;
                          }
                          }else if 
                              (event.getAction()==android.view.MotionEvent.ACTION_UP){
                              mp1.stop();
                              mp1.reset();
                              mp1.release();
                              v.setPressed(false);
                              clickCounter();
                              return false;
                              }                   
                         break;

Solution

Dodging your question for a quick comment:

Why don't you combine these

if(intLoop ==0){
                          mp1=MediaPlayer.create(MainClass.this, R.raw.item1);
                          mp1.start();
                          v.setPressed(true);
                          return true;
                      }
                      if(intLoop == 1){
                          mp1=MediaPlayer.create(MainClass.this, R.raw.item1);
                          mp1.start();
                          mp1.setLooping(true);
                          v.setPressed(true);
                          return true;
                      }


into this?

mp1=MediaPlayer.create(MainClass.this, R.raw.item1);
                      mp1.start();
                      if(intLoop == 1){
                          mp1.setLooping(true);
                      }
                      v.setPressed(true);
                      return true;


And, as Vogel612 mentions, is there any reason intLoop is not a boolean?

I think that if the rest of your code looks like that, simply combining the functionality like this might fix the idea that the listener is doing a lot.

Code Snippets

if(intLoop ==0){
                          mp1=MediaPlayer.create(MainClass.this, R.raw.item1);
                          mp1.start();
                          v.setPressed(true);
                          return true;
                      }
                      if(intLoop == 1){
                          mp1=MediaPlayer.create(MainClass.this, R.raw.item1);
                          mp1.start();
                          mp1.setLooping(true);
                          v.setPressed(true);
                          return true;
                      }
mp1=MediaPlayer.create(MainClass.this, R.raw.item1);
                      mp1.start();
                      if(intLoop == 1){
                          mp1.setLooping(true);
                      }
                      v.setPressed(true);
                      return true;

Context

StackExchange Code Review Q#58517, answer score: 3

Revisions (0)

No revisions yet.