Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

Not only can you have your class implement the interface, but you can also use an onClick property in your XML. See onClick in XML vs. onClickListener onClick in XML vs. onClickListener

In onCreate:

start = (Button) findViewById(R.id.startButton);

New method in your class:

public void startButtonPress(View view) {
 mp.start();
}

In your XML:

<button .... onClick="startButtonPress" />

Additionally, you should look over your indentation and spacing.

This part of your code:

 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();} 

Is more readable as:

 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();
 }

Additionally:

  • Think about the users of your app, do all of them have a /Music/maine.mp3 file? No. I don't know what your main goal with this app is though, but playing only one specific file that has to be at a certain place hopefully isn't it.
  • Again, think about the users, can they read the output from e.printStackTrace(); with adb logcat? No. I'd recommend showing a Toast or a Dialog there as well.

Not only can you have your class implement the interface, but you can also use an onClick property in your XML. See onClick in XML vs. onClickListener

In onCreate:

start = (Button) findViewById(R.id.startButton);

New method in your class:

public void startButtonPress(View view) {
 mp.start();
}

In your XML:

<button .... onClick="startButtonPress" />

Additionally, you should look over your indentation and spacing.

This part of your code:

 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();} 

Is more readable as:

 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();
 }

Additionally:

  • Think about the users of your app, do all of them have a /Music/maine.mp3 file? No. I don't know what your main goal with this app is though, but playing only one specific file that has to be at a certain place hopefully isn't it.
  • Again, think about the users, can they read the output from e.printStackTrace(); with adb logcat? No. I'd recommend showing a Toast or a Dialog there as well.

Not only can you have your class implement the interface, but you can also use an onClick property in your XML. See onClick in XML vs. onClickListener

In onCreate:

start = (Button) findViewById(R.id.startButton);

New method in your class:

public void startButtonPress(View view) {
 mp.start();
}

In your XML:

<button .... onClick="startButtonPress" />

Additionally, you should look over your indentation and spacing.

This part of your code:

 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();} 

Is more readable as:

 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();
 }

Additionally:

  • Think about the users of your app, do all of them have a /Music/maine.mp3 file? No. I don't know what your main goal with this app is though, but playing only one specific file that has to be at a certain place hopefully isn't it.
  • Again, think about the users, can they read the output from e.printStackTrace(); with adb logcat? No. I'd recommend showing a Toast or a Dialog there as well.
Source Link
Simon Forsberg
  • 59.7k
  • 9
  • 157
  • 311

Not only can you have your class implement the interface, but you can also use an onClick property in your XML. See onClick in XML vs. onClickListener

In onCreate:

start = (Button) findViewById(R.id.startButton);

New method in your class:

public void startButtonPress(View view) {
 mp.start();
}

In your XML:

<button .... onClick="startButtonPress" />

Additionally, you should look over your indentation and spacing.

This part of your code:

 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();} 

Is more readable as:

 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();
 }

Additionally:

  • Think about the users of your app, do all of them have a /Music/maine.mp3 file? No. I don't know what your main goal with this app is though, but playing only one specific file that has to be at a certain place hopefully isn't it.
  • Again, think about the users, can they read the output from e.printStackTrace(); with adb logcat? No. I'd recommend showing a Toast or a Dialog there as well.
lang-java

AltStyle によって変換されたページ (->オリジナル) /