Reputation: 307
I'm currently working on my third Android project for mass release. My first two were relatively basic compared to my current application, which is by far my most complex work. As such, I've been having to deal with more listeners then I'm used to.
Specifically, the portion of my application with deals with playing a web stream in MediaPlayer is already using three listeners.
A listener for a spinner:
stationSpinner.setOnItemSelectedListener(new OnItemSelectedListener() {
String newStreamUrl;
@Override
public void onItemSelected(AdapterView<?> parentView, View selectedItemView, int position, long id) {
switch (position) {
// Based on the user's selection, change the URL to match the appropriate station and stream quality.
case 0:
// 128kb 89.7 stream
// Default case, always executed on activity creation.
newStreamUrl = "defaultStreamUrl";
changeStream(newStreamUrl);
break;
case 1:
// 320kb stream
newStreamUrl = "URL1";
changeStream(newStreamUrl);
break;
case 2:
// 128kb Stream 2
newStreamUrl = "URL2";
changeStream(newStreamUrl);
break;
case 3:
// 320kb Steam 2
newStreamUrl = "URL3";
changeStream(newStreamUrl);
}
}
@Override
public void onNothingSelected(AdapterView<?> parentView) {
}
});
A listener for a progressDialog:
pd = ProgressDialog.show(this, "Loading...", "Buffering Stream", true, true, new DialogInterface.OnCancelListener() {
@Override
public void onCancel(DialogInterface dialog) {
pd.dismiss();
mp.reset();
}
});
And a listener to trigger playback when buffering is complete:
mp.setOnPreparedListener(new OnPreparedListener() {
@Override
public void onPrepared(MediaPlayer mp) {
// When the stream is buffered, kill prompt and start playing automatically.
pd.dismiss();
mp.start();
Log.i(TAG, "Stream playback started.");
}
});
...and I'm not even halfway finished with implementing listeners to deal with all possible situations.
From my current knowledge, a listener has to either be defined anonymously (as I have done above), or written as a separate class which implements the listener and then instantiated in the desired class. Maybe it's just me, but I think defining these anonymously clutters my code and obscures the logic behind the activity. However, since I'm really only using these listeners in this class, it seems like a waste to move them to their own separate file, as it will eat away the namespace of my package.
I'm wondering what the best practice is for a situation like this. Is there a well defined rule regarding listeners or is it just up to the preference of the developer? I'm trying to conform this project as close to best coding practices as I can, since I'm new to writing Android applications, so hopefully I can learn something from this. Any thoughts?
My entire code for context if necessary:
public class StreamActivity extends Activity {
static private MediaPlayer mp;
static ProgressDialog pd;
static String streamUrl = "defaultStreamURL"; // Default value is 128kb/s stream.
private static final String TAG = StreamActivity.class.getName(); // Tag constant for logging purposes
@Override
public boolean onCreateOptionsMenu(Menu menu) {
// Inflate the menu; this adds items to the action bar if it is present.
getMenuInflater().inflate(R.menu.stream, menu);
return true;
}
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.stream_layout);
// Build selection spinner
Spinner stationSpinner = (Spinner)findViewById(R.id.station_spinner);
ArrayAdapter<CharSequence> stationAdapter = ArrayAdapter.createFromResource(this, R.array.station_string_array, android.R.layout.simple_spinner_item);
stationAdapter.setDropDownViewResource(android.R.layout.simple_spinner_dropdown_item);
stationSpinner.setAdapter(stationAdapter);
// Set spinner decision logic
stationSpinner.setOnItemSelectedListener(new OnItemSelectedListener() {
String newStreamUrl;
@Override
public void onItemSelected(AdapterView<?> parentView, View selectedItemView, int position, long id) {
switch (position) {
// Based on the user's selection, change the URL to match the appropriate station and stream quality.
case 0:
// 128kb 89.7 stream
// Default case, always executed on activity creation.
newStreamUrl = "defaultStreamUrl";
changeStream(newStreamUrl);
break;
case 1:
// 320kb stream
newStreamUrl = "URL1";
changeStream(newStreamUrl);
break;
case 2:
// 128kb Stream 2
newStreamUrl = "URL2";
changeStream(newStreamUrl);
break;
case 3:
// 320kb Steam 2
newStreamUrl = "URL3";
changeStream(newStreamUrl);
}
}
@Override
public void onNothingSelected(AdapterView<?> parentView) {
}
});
// Build audio player using default settings.
mp = buildAudioPlayer();
}
/**
* Builds and returns a configured, unprepared MediaPlayer.
*/
public MediaPlayer buildAudioPlayer() {
// Build MediaPlayer
mp = new MediaPlayer();
try {
mp.reset();
mp.setAudioStreamType(AudioManager.STREAM_MUSIC);
mp.setDataSource(streamUrl);
} catch (IllegalArgumentException e) {
Log.e(TAG, "Caught IllegalArgumentException: ");
e.printStackTrace();
} catch (IllegalStateException e) {
Log.e(TAG, "Caught IllegalStateException: ");
e.printStackTrace();
} catch (SecurityException e) {
Log.e(TAG, "Caught SecurityException: ");
e.printStackTrace();
} catch (IOException e) {
Log.e(TAG, "Caught IOException: ");
e.printStackTrace();
}
pd = new ProgressDialog(this);
return mp;
}
protected void changeStream(String newStreamUrl) {
streamUrl = newStreamUrl;
// Stop stream if it is currently playing to prevent state exceptions
if (mp.isPlaying()) {
Log.i(TAG, "Stream source changed by user. Rebuilding stream.");
Log.i(TAG, "Stream playback stopped.");
mp.stop();
}
// Rebuild player with new stream URL.
mp.reset();
mp = buildAudioPlayer();
}
/**
* Stops audio, drops connection to stream, and returns Media Player to an unprepared state. Called by a button onClick event.
* @param v Button pressed by user.
*/
public void stopAudio(View v) {
mp.stop();
Log.i(TAG, "Stream playback stopped.");
}
/**
* Pauses audio with no change to connection or Media Player. Called by a button onClick event.
* @param v Button pressed by user.
*/
public void pauseAudio(View v) {
mp.pause();
Log.i(TAG, "Stream playback paused.");
}
/**
* Prepares Media Player asynchronously. Displays prompt while buffering and automatically starts when finished.
* @param v Button pressed by user.
*/
public void playAudio(View v) {
// If we are paused, resume playback without rebuffering.
if (mp.isPlaying()) {
mp.start();
} else {
// If audio is NOT playing, we need to prepare and buffer.
try {
mp.setOnPreparedListener(new OnPreparedListener() {
@Override
public void onPrepared(MediaPlayer mp) {
// When the stream is buffered, kill prompt and start playing automatically.
pd.dismiss();
mp.start();
Log.i(TAG, "Stream playback started.");
}
});
// Prepares stream without blocking UI Thread
mp.prepareAsync();
} catch (IllegalStateException e) {
Log.e(TAG, "Caught IllegalStateException when preparing: ");
e.printStackTrace();
}
// Stop user input while buffering by displaying ProgressDialog
pd.setCancelable(true);
pd.setCanceledOnTouchOutside(false);
pd = ProgressDialog.show(this, "Loading!", "Buffering...", true, true, new DialogInterface.OnCancelListener() {
@Override
public void onCancel(DialogInterface dialog) {
pd.dismiss();
mp.reset();
}
});
}
}
}
Upvotes: 0
Views: 501
Reputation: 691685
It clutters your code because you're making them too long. Reduce them to two or three lines max, and it will be more readable. For example:
stationSpinner.setOnItemSelectedListener(new OnItemSelectedListener() {
@Override
public void onItemSelected(AdapterView<?> parentView,
View selectedItemView,
int position,
long id) {
changeUrl(position);
}
@Override
public void onNothingSelected(AdapterView<?> parentView) {
}
};
...
private void changeUrl(int spinnerPosition) {
String newStreamUrl;
switch (position) {
case 0:
// 128kb 89.7 stream
// Default case, always executed on activity creation.
newStreamUrl = "defaultStreamUrl";
changeStream(newStreamUrl);
break;
case 1:
// 320kb stream
newStreamUrl = "URL1";
changeStream(newStreamUrl);
break;
case 2:
// 128kb Stream 2
newStreamUrl = "URL2";
changeStream(newStreamUrl);
break;
case 3:
// 320kb Steam 2
newStreamUrl = "URL3";
changeStream(newStreamUrl);
}
}
}
Or, if it's still too long, you can implement them as non-anonymous inner classes. For example:
stationSpinner.setOnItemSelectedListener(new StationSpinnerListener());
...
private class StationSpinnerListener implements OnItemSelectedListener {
@Override
public void onItemSelected(AdapterView<?> parentView,
View selectedItemView,
int position,
long id) {
String newStreamUrl;
switch (position) {
case 0:
// 128kb 89.7 stream
// Default case, always executed on activity creation.
newStreamUrl = "defaultStreamUrl";
changeStream(newStreamUrl);
break;
case 1:
// 320kb stream
newStreamUrl = "URL1";
changeStream(newStreamUrl);
break;
case 2:
// 128kb Stream 2
newStreamUrl = "URL2";
changeStream(newStreamUrl);
break;
case 3:
// 320kb Steam 2
newStreamUrl = "URL3";
changeStream(newStreamUrl);
}
}
}
@Override
public void onNothingSelected(AdapterView<?> parentView) {
}
}
Upvotes: 1