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

Android audio player

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

Problem

I want to implement basic audio functions like play, stop and pause. I have stuffed all the code inside onCreate method. Is this best practise to follow?

public class MainActivity extends Activity {  
        Button start,pause,stop;  
        @Override  
        protected void onCreate(Bundle savedInstanceState) {  
            super.onCreate(savedInstanceState);  
            setContentView(R.layout.activity_main);  

            start=(Button)findViewById(R.id.button1);  
            pause=(Button)findViewById(R.id.button2);  
            stop=(Button)findViewById(R.id.button3);  
            //creating media player  
            final MediaPlayer mp=new MediaPlayer();  
            try{  
                    //you can change the path, here path is external directory(e.g. sdcard) /Music/maine.mp3  
            mp.setDataSource(Environment.getExternalStorageDirectory().getPath()+"/Music/maine.mp3");  

            mp.prepare();  
            }catch(Exception e){e.printStackTrace();}  

            start.setOnClickListener(new OnClickListener() {  
                @Override  
                public void onClick(View v) {  
                    mp.start();  
                }  
            });  
            pause.setOnClickListener(new OnClickListener() {  
                @Override  
                public void onClick(View v) {  
                    mp.pause();  
                }  
            });  
            stop.setOnClickListener(new OnClickListener() {  
                @Override  
                public void onClick(View v) {  
                    mp.stop();  
                }  
            });  
        }  
    }

Solution

You could let your class implement OnClickListener

public class MainActivity extends Activity implements OnClickListener{
    ...
}


and then use the onClickfunction like this:

public void onClick(View v)
{
    switch (v.getId())
    {
        case R.id.button1:
            mp.start();  
            break;
        case R.id.button2:
            mp.pause();  
            break;
        case R.id.button3:
            mp.stop();  
            break;
        default:
            break;
    }
}


That way you have all actions that happen on a press of a button in the same place in your program.

You should also change the ID to something more meaningful, eg. play, pause, stop.
This makes your code far more maintainable and easier to understand which Button does what just by skimming the code of the MainActivity(which you also might want to rename to something like MediaPlayerActivity incase you decide to add additional Activity classes, for example to select a new song.

Additionally I would advise you to change the visibility of the Buttons to private, rather than default.

Code Snippets

public class MainActivity extends Activity implements OnClickListener{
    ...
}
public void onClick(View v)
{
    switch (v.getId())
    {
        case R.id.button1:
            mp.start();  
            break;
        case R.id.button2:
            mp.pause();  
            break;
        case R.id.button3:
            mp.stop();  
            break;
        default:
            break;
    }
}

Context

StackExchange Code Review Q#60775, answer score: 6

Revisions (0)

No revisions yet.