4
\$\begingroup\$

I wanted to create a very basic application for Android that plays an audio file based on a URL input. I would like it to be reviewed based on good programming practices. This is the first time that I have used the MediaPlayer class. It works as I would like it to, but I am sure my code can be optimized and improved.

Following is my code for the MediaPlayerActivity class that I have built:

public class MediaPlayerActivity extends AppCompatActivity {
 TextView tvArtistName, tvSongName;
 ImageView ivPlay, ivPrev, ivNext, ivPause;
 MediaPlayer mediaPlayer = new MediaPlayer();
 private double startTime = 0;
 private double finalTime = 0;
 private Handler myHandler = new Handler();
 ;
 private int forwardTime = 5000;
 private int backwardTime = 5000;
 private SeekBar seekbar;
 public static int oneTimeOnly = 0;
 ActionBar actionBar;
 @Override
 protected void onCreate(Bundle savedInstanceState) {
 super.onCreate(savedInstanceState);
 setContentView(R.layout.activity_media_player);
 actionBar = getSupportActionBar();
 actionBar.hide();
 tvArtistName = (TextView) findViewById(R.id.tvArtistName);
 tvSongName = (TextView) findViewById(R.id.tvSongName);
 ivPlay = (ImageView) findViewById(R.id.iv_play);
 ivPause = (ImageView) findViewById(R.id.iv_pause);
 ivNext = (ImageView) findViewById(R.id.iv_next);
 ivPrev = (ImageView) findViewById(R.id.iv_prev);
 seekbar = (SeekBar) findViewById(R.id.seekBar);
 seekbar.setClickable(false);
 Intent intent = getIntent();
 if (intent.getExtras() != null) {
 final String url = intent.getStringExtra("url");
 String song = intent.getStringExtra("song");
 String artist = intent.getStringExtra("artist");
 tvArtistName.setText(artist);
 tvSongName.setText(song);
 mediaPlayer.setAudioStreamType(AudioManager.STREAM_MUSIC);
 mediaPlayer = MediaPlayer.create(getApplicationContext(), Uri.parse(url));
 ivPlay.setOnClickListener(new View.OnClickListener() {
 @Override
 public void onClick(View view) {
 // Toast.makeText(getApplicationContext(), "Playing sound", Toast.LENGTH_SHORT).show();
 if (mediaPlayer.isPlaying()) {
 mediaPlayer.stop();
 mediaPlayer.reset();
 }
 mediaPlayer.setOnCompletionListener(new MediaPlayer.OnCompletionListener() {
 @Override
 public void onCompletion(MediaPlayer mediaPlayer) {
 mediaPlayer.stop();
 mediaPlayer.reset();
 }
 });
 mediaPlayer.start();
 finalTime = mediaPlayer.getDuration();
 startTime = mediaPlayer.getCurrentPosition();
 if (oneTimeOnly == 0) {
 seekbar.setMax((int) finalTime);
 oneTimeOnly = 1;
 }
 seekbar.setProgress((int) startTime);
 myHandler.postDelayed(UpdateSongTime, 100);
 ivPlay.setVisibility(View.GONE);
 ivPause.setVisibility(View.VISIBLE);
 }
 });
 ivPause.setOnClickListener(new View.OnClickListener() {
 @Override
 public void onClick(View view) {
 Toast.makeText(getApplicationContext(), "Pausing sound", Toast.LENGTH_SHORT).show();
 mediaPlayer.pause();
 ivPause.setVisibility(View.GONE);
 ivPlay.setVisibility(View.VISIBLE);
 }
 });
 ivNext.setOnClickListener(new View.OnClickListener() {
 @Override
 public void onClick(View view) {
 int temp = (int) startTime;
 if ((temp + forwardTime) <= finalTime) {
 startTime = startTime + forwardTime;
 mediaPlayer.seekTo((int) startTime);
 //Toast.makeText(getApplicationContext(),"You have Jumped forward 5 seconds",Toast.LENGTH_SHORT).show();
 } else {
 //Toast.makeText(getApplicationContext(),"Cannot jump forward 5 seconds",Toast.LENGTH_SHORT).show();
 }
 }
 });
 ivPrev.setOnClickListener(new View.OnClickListener() {
 @Override
 public void onClick(View view) {
 int temp = (int) startTime;
 if ((temp - backwardTime) > 0) {
 startTime = startTime - backwardTime;
 mediaPlayer.seekTo((int) startTime);
 //Toast.makeText(getApplicationContext(),"You have Jumped backward 5 seconds",Toast.LENGTH_SHORT).show();
 } else {
 //Toast.makeText(getApplicationContext(),"Cannot jump backward 5 seconds",Toast.LENGTH_SHORT).show();
 }
 }
 });
 }
 }
 @Override
 public void onBackPressed() {
 super.onBackPressed();
 mediaPlayer.release();
 mediaPlayer = null;
 seekbar.setProgress(0);
 startTime = 0;
 finalTime = 0;
 }
 private Runnable UpdateSongTime = new Runnable() {
 public void run() {
 if (mediaPlayer!=null)
 {
 startTime = mediaPlayer.getCurrentPosition();
 seekbar.setProgress((int) startTime);
 myHandler.postDelayed(this, 100);
 }
 }
 };
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Nov 18, 2017 at 15:54
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

It looks good to me. Sensible identifiers, usual idioms.

After defining myHandler, delete the 2nd ; semicolon.

You probably want a boolean for oneTimeOnly.

Your "Pausing sound" toast is nice enough, but consider defining a TextView where you can flash messages, as that can give you greater thread scheduling control. A toast can block other threads from running for a moment, which will sometimes be important to an app.

If intent.getExtras() is null, it seems the caller did the Wrong Thing, so you might want to give up and signal fatal error at that point.

answered Nov 18, 2017 at 21:39
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.