[npr-android-app] push by justinfr...@gmail.com - Code review changes on 2012-01-06 23:17 GMT

[npr-and...@googlecode.com]

Revision: 7cf2352b5c3c
Author: Justin Friberg <justin...@hotmail.com>
Date: Fri Jan 6 15:16:14 2012
Log: Code review changes

http://code.google.com/p/npr-android-app/source/detail?r=7cf2352b5c3c

Modified:
/Npr/src/org/npr/android/news/PlaybackService.java
/Npr/src/org/npr/android/news/PlaylistView.java
/Npr/src/org/npr/android/news/StationListActivity.java
/Npr/src/org/npr/android/util/StationCache.java
/Npr/src/org/npr/api/Station.java
/Npr/src/org/npr/api/Story.java

=======================================
--- /Npr/src/org/npr/android/news/PlaybackService.java Fri Jan 6 11:10:49
2012
+++ /Npr/src/org/npr/android/news/PlaybackService.java Fri Jan 6 15:16:14
2012
@@ -48,6 +48,7 @@
import java.net.URL;
import java.net.URLConnection;
import java.net.UnknownHostException;
+import java.sql.Connection;
import java.util.List;

public class PlaybackService extends Service implements
@@ -88,7 +89,9 @@
public static final String EXTRA_SEEK_TO = SERVICE_PREFIX + "SEEK_TO";
public static final String EXTRA_IS_PLAYING = SERVICE_PREFIX
+ "IS_PLAYING";
public static final String EXTRA_IS_PREPARED = SERVICE_PREFIX
+ "IS_PREPARED";
- public static final String EXTRA_ERROR_MESSAGE = SERVICE_PREFIX
+ "ERROR_MESSAGE";
+
+ public static final String EXTRA_ERROR = SERVICE_PREFIX + "ERROR";
+ public static enum PLAYBACK_SERVICE_ERROR {Connection, Playback}

private MediaPlayer mediaPlayer;
private boolean isPrepared = false;
@@ -671,8 +674,7 @@
Log.e(LOG_TAG, "Media player increment error count:" + errorCount);
if (errorCount >= ERROR_RETRY_COUNT) {
Intent intent = new Intent(SERVICE_ERROR_NAME);
- intent.putExtra(EXTRA_ERROR_MESSAGE,
- getString(R.string.msg_playback_error));
+ intent.putExtra(EXTRA_ERROR,
PLAYBACK_SERVICE_ERROR.Playback.ordinal());
getApplicationContext().sendBroadcast(intent);
}
}
@@ -684,8 +686,7 @@
" and trying again in 30 seconds.");

Intent intent = new Intent(SERVICE_ERROR_NAME);
- intent.putExtra(EXTRA_ERROR_MESSAGE,
- getString(R.string.msg_playback_connection_error));
+ intent.putExtra(EXTRA_ERROR,
PLAYBACK_SERVICE_ERROR.Connection.ordinal());
getApplicationContext().sendBroadcast(intent);

// If a stream, increment since it could be bad
=======================================
--- /Npr/src/org/npr/android/news/PlaylistView.java Fri Jan 6 11:10:49 2012
+++ /Npr/src/org/npr/android/news/PlaylistView.java Fri Jan 6 15:16:14 2012
@@ -503,11 +503,15 @@
@Override
public void onReceive(Context context, Intent intent) {
Log.d(LOG_TAG, "Playback error received - toasting message");
- String error =
intent.getStringExtra(PlaybackService.EXTRA_ERROR_MESSAGE);
- if (error == null || error.length() == 0) {
- error = "Unknown error occurred.";
- }
- Toast.makeText(context, error, Toast.LENGTH_LONG).show();
+ String message = "Unknown error occurred.";
+
+ int error = intent.getIntExtra(PlaybackService.EXTRA_ERROR, -1);
+ if (error ==
PlaybackService.PLAYBACK_SERVICE_ERROR.Playback.ordinal()) {
+ message = context.getString(R.string.msg_playback_error);
+ } else if (error ==
PlaybackService.PLAYBACK_SERVICE_ERROR.Connection.ordinal()) {
+ message = context.getString(R.string.connection_error);
+ }
+ Toast.makeText(context, message, Toast.LENGTH_LONG).show();
}
}

=======================================
--- /Npr/src/org/npr/android/news/StationListActivity.java Fri Jan 6
11:10:49 2012
+++ /Npr/src/org/npr/android/news/StationListActivity.java Fri Jan 6
15:16:14 2012
@@ -114,15 +114,15 @@
lv.setOnItemClickListener(StationListActivity.this);
lv.setOnItemLongClickListener(StationListActivity.this);

- if (mode != Mode.favoriteStations) {
- loadFromQuery(queryUrl);
- } else {
+ if (mode == Mode.favoriteStations) {
TextView presetInstructions = (TextView)
findViewById(R.id.preset_instructions);
if (favoriteStationsRepository.getItemCount() > 0) {
presetInstructions.setVisibility(View.VISIBLE);
} else {
presetInstructions.setVisibility(View.GONE);
}
+ } else {
+ loadFromQuery(queryUrl);
}

Button locateButton = (Button) findViewById(R.id.StationSearchButton);
@@ -322,47 +322,52 @@
startActivityWithoutAnimation(i);
}

- private void setPreset(int item) {
- if (item == 10) {
- Station station = listAdapter.getItem(selectedPosition);
- favoriteStationsRepository.removePreset(station.getId());
- } else {
- favoriteStationsRepository.setPreset(
- listAdapter.getItem(selectedPosition), Integer.toString(item +
1));
- }
+ private void removePreset() {
+ Station station = listAdapter.getItem(selectedPosition);
+ favoriteStationsRepository.removePreset(station.getId());
listAdapter.notifyDataSetChanged();
}
+
+ private void setPreset(int presetNumberSelected) {
+ favoriteStationsRepository.setPreset(
+ listAdapter.getItem(selectedPosition),
Integer.toString(presetNumberSelected + 1));
+ listAdapter.notifyDataSetChanged();
+ }

@Override
public boolean onItemLongClick(AdapterView<?> parent, View view, int
position, long id) {

- if (mode == Mode.favoriteStations) {
- selectedPosition = position;
-
- int presetNumber = 10;
- FavoriteStationEntry favoriteStationEntry =
- favoriteStationsRepository.getFavoriteStationForStationId(
- listAdapter.getItem(position).getId());
- if (favoriteStationEntry != null &&
- favoriteStationEntry.preset != null &&
- favoriteStationEntry.preset.length() > 0) {
- presetNumber = Integer.parseInt(favoriteStationEntry.preset) - 1;
- }
- AlertDialog.Builder builder = new AlertDialog.Builder(this);
- builder.setTitle("Pick a preset");
- builder.setSingleChoiceItems(presets, presetNumber, new
DialogInterface.OnClickListener() {
- public void onClick(DialogInterface dialog, int item) {
+ if (mode != Mode.favoriteStations) {
+ return false;
+ }
+ selectedPosition = position;
+
+ int presetNumber = 10;
+ FavoriteStationEntry favoriteStationEntry =
+ favoriteStationsRepository.getFavoriteStationForStationId(
+ listAdapter.getItem(position).getId());
+ if (favoriteStationEntry != null &&
+ favoriteStationEntry.preset != null &&
+ favoriteStationEntry.preset.length() > 0) {
+ presetNumber = Integer.parseInt(favoriteStationEntry.preset) - 1;
+ }
+ AlertDialog.Builder builder = new AlertDialog.Builder(this);
+ builder.setTitle("Pick a preset");
+ builder.setSingleChoiceItems(presets, presetNumber, new
DialogInterface.OnClickListener() {
+ public void onClick(DialogInterface dialog, int item) {
+ if (item == 10) {
+ removePreset();
+ } else {
setPreset(item);
- dialog.dismiss();
- }
- });
-
- AlertDialog alert = builder.create();
- alert.show();
-
- return true;
- }
- return false;
+ }
+ dialog.dismiss();
+ }
+ });
+
+ AlertDialog alert = builder.create();
+ alert.show();
+
+ return true;
}

private String populateLocalStationParams(Map<String, String> params) {
=======================================
--- /Npr/src/org/npr/android/util/StationCache.java Fri Jan 6 11:10:49 2012
+++ /Npr/src/org/npr/android/util/StationCache.java Fri Jan 6 15:16:14 2012
@@ -26,8 +26,10 @@
new Hashtable<String, StationEntry>();

public static void addAll(List<Station> stations) {
- for (Station station : stations) {
- stationCache.put(station.getId(), new StationEntry(station));
+ synchronized (stationCache) {
+ for (Station station : stations) {
+ stationCache.put(station.getId(), new StationEntry(station));
+ }
}
}

@@ -48,6 +50,8 @@
}

public static void clear() {
- stationCache.clear();
+ synchronized (stationCache) {
+ stationCache.clear();
+ }
}
}
=======================================
--- /Npr/src/org/npr/api/Station.java Fri Jan 6 11:10:49 2012
+++ /Npr/src/org/npr/api/Station.java Fri Jan 6 15:16:14 2012
@@ -48,6 +48,7 @@
private final String image;
private final HashMap<String, String> identiferAudioUrlByType;

+ private static final Pattern callLettersPattern =
Pattern.compile("[KW][A-Z][A-Z][A-Z]");
public static final String[] stationListBySize = new
String[]{"WNYC","KPCC","KUSD","KPCV","KQED","WAMU","WBUR","WBEZ","WBEQ","WBEW","KNOW","KSJN","KCMP","KMSE","KBPN","KCCD","KLNI","KNBJ","KNCM","KNGA","KNSE","KNSR","KNSW","KNTN","KNWF","KZSE","WGGL","WIRN","WLSN","WSCN","KCCM","KCMF","KLSE","KRSD","KRSW","KSJR","WIRR","WMLS","WSCD","WHYY","KCRW","KCRI","KCRU","KUOW","KOAC","KOAB","KOAP","KOBK","KOPB","KRBM","KTMK","KTVR","KMHD","WABE","WEVN","WSUF","WUOM","WFUM","WVGR","KJZZ","KBAQ","WKWM","WLRN","KPLU","KPLI","KVIX","KERA","KUHF","KXOT","WUSF","KPBS","KQVO","WUNC","WBUX","WRQM","WUND","WURI","WAMC","WAMK","WAMQ","WCAN","WOSR","WRUN","KCFR","KCFC","KCFP","KKPC","KPRE","KPRH","KPRN","KPRU","KPYR","KVOV","WGBH","WCAI","WNAN","WNCK","WZAI","WMEH","WMED","WMEF","WMEM","WMEP","WMEW","WSUI","KDWI","KOWI","KSUI","KTPR","KUNZ","KWOI","WOI","KUT","KUTX","KWMU","WVPS","WBTN","WNCH","WOXR","WRVT","WVPA","WVPR","WVTI","WVTQ","WWPV","KXJZ","KXPR","KKTO","KQNC","KUOP","KXJS","KXSR","WSHU","WFAE","WFHE","WLTR","WEPR","WHMC","WJWJ","WLJK","WNSC","WRJA","WSCI","WFCR","WNPR","WEDW","WPKT","WRLI","KQEI","WCPN","WYPR","WYPF","KCUR","WERN","WHND","WHRM","WHSA","WLSU","WPNE","WSSW","WUEC","WVSS","KUWS","WHA","WHBM","WHDI","WHHI","WHID","WHLA","WHWC","WLBL","WRFW","WRST","WMFE","KUNC","KRNC","KDMR","KDUB","KRNI","KUNY","WHRS","WPLN","WTML","WDUQ","WEVC","WEVH","WEVJ","WEVO","WEVS","WYPM","WITF","WHRV","WHRO","WFFC","WISE","WVTF","WVTR","WVTW","WVTU","WWVT","WCVE","WCNV","WMVE","WVXU","WMUB","WKRJ","WKSU","WKRW","WKSV","WNRK","WDET","WFYI","KHPR","KANO","KIPO","KKUA","KWSU","KRFA","KFAE","KLWS","KMWS","KNWO","KNWP","KNWR","KNWV","KNWY","KQWS","KWWS","KZAZ","KSTX","KTXI","KNPR","KLNR","KSGU","KTPH","KWPR","KUER","WRVO","WRCU","WRVD","WRVJ","WRVN","WSUC","WGCU","WMKO","WJCT","WFSU","WFSL","WFSQ","WFSW","WKAR","WJSP","WSVH","WUGA","WABR","WACG","WATY","WGPB","WJWV","WMUM","WNGH","WNGU","WPPR","WUNV","WUWG","WWET","WWIO","WXVS","WVPN","WAUA","WVEP","WVNP","WVPB","WVPG","WVPM","WVPW","WVWV","KUAZ","WPBI","WMPN","WMAB","WMAE","WMAH","WMAO","WMAU","WMAV","WMAW","WUWM","WUOT","WXXI","WRUR","WXXY","WSKG","WSQA","WSQC","WSQE","WSQG","WSQX","WBFO","WOLN","WUBJ","WFDD","WBHM","WFPL","KPBX","KIBX","KSFC","WOSU","WOSB","WOSE","WOSP","WOSV","KLCC","KLBR","KLCO","KLFO","KLFR","KMPQ","WCQS","WFQS","WYQS","WCBE","KNAU","KGHR","KNAA","KNAD","KNAG","KNAQ","KPUB","KUNM","KANU","KANH","KANV","KUWR","KBUW","KDUW","KSUW","KUWA","KUWC","KUWD","KUWG","KUWJ","KUWL","KUWN","KUWP","KUWT","KUWX","KUWY","KUWZ","WOUB","WOUC","WOUH","WOUL","WOUZ","WQCS","WKNO","WKNP","KUAR","KLRE","WCMU","WCMB","WCML","WCMW","WCMZ","WUCX","WWCM","KWGS","KWTU","KBSW","KBSJ","KBSQ","KBSX","KBSY","KBSK","KBSS","KBSU","WVIA","WVYA","KJZA","KXLC","WHAD","WMEA","KUCV","KCNE","KHNE","KLNE","KMNE","KPNE","KRNE","KTNE","KXNE","KVPR","KPRX","WFIU","WUFT","WJUF","KSOR","KAGI","KLDD","KLMF","KNCA","KNHT","KNSQ","KNYR","KSBA","KSKF","KSMF","KSRG","KSRS","KRCC","KECC","WVPE","WUWF","KGOU","KROU","WWNO","KTLN","WNIJ","WNIE","WNIQ","WNIW","WNIU","KUAF","KMUW","WBLQ","WRNI","WRKF","WSLU","WSLJ","WSLL","WSLO","WXLB","WXLG","WXLH","WXLQ","WXLU","KVCR","WLRH","KCLU","KALW","WUTC","KIOS","WMRA","WMRL","WMRY","WILL","WSCL","WSDL","WGTE","WGBE","WGDE","WGLE","KCHO","KFPR","KUNR","KNCC","KDAQ","KBSA","KLDN","KLSA","WPSU","WPSX","WHQR","WUKY","WIAA","WIAB","WICA","WICV","WYSO","WUAL","WAPR","WQPR","WYSU","KEMC","KYPR","KBMC","KPRQ","WBAA","WBNI","WBOI","WCKZ","WEKU","WEKF","WEKH","KSMU","KSMS","KSMW","KBHE","KCSD","KDSD","KESD","KPSD","KQSD","KTSD","KZSD","KCBX","KSBX","KUFM","KAPC","KUFN","KUHM","KUKL","KRWG","WETS","WTEB","WBJD","WKNS","WZNB","WKYU","WDCL","WKPB","WKUE","KBIA","WQLN","KCPW","KHCC","KHCD","KHCT","WAER","WGVU","WGVS","WSIU","WUSI","WVSI","WUIS","WIPA","WTSU","WRWA","WTJB","WMUK","KUSP","KBDH","KHSU","WGLT","WANC","WCEL","WXPN","WCBU","KCND","KDPR","KMPR","KPPR","KPRJ","WBST","WBSB","WBSH","WBSJ","WBSW","WKMS","WNIN","KRPS","KEDT","KVRT","WHIL","WEMU","WIUM","WIUW","KAZU","WNJT","WNJB","WNJM","WNJN","WNJP","WNJS","WNJZ","KCCU","KLCU","KMCU","KOCU","KYCU","KZCU","WVIK","KAMU","WDIY","WXLV","WNED","KAWC","KANZ","KJJP","KTOT","KTXP","KZAN","KZNA","WNMU","KWIT","KOJI","KRVS","WNKU","KOHM","KOSU","KOSN","KXCV","KRNW","KMBH","KHID","WFUV","WETA","WGMS","KEDM","KUSU","KUSR","KWBU","WXPR","WXPW","KASU","WRTI","WJAZ","WRTQ","WRTX","WRTY","KVLU","WKGC","WMKY","WOCS","KENW","KMTH","KSUT","KUTE","WFIT","WBLV","WBLU","KZYX","KZYZ","KAJX","KCJX","KPCW","KCUA","KANW","KNLK","WBGO","KRCU","KSEF","KPRG","KTBG","WQUB","WFSS","WYEP","WUCF","KAXE","WNCW","WQED","WQEJ","WDAV","WWFM","WWNJ","WWCJ","WWPJ","WBJB","KRCB","WCLK","WRVS","WTMD","WMNF","WESM","WMHT","WEXT","WRHV","WEAA","WCNY","WJNY","WUNY","WJAB","KETR","WBPR","WFPB","WNEF","WUMB","KUNV","WHDD","WLPR","WYPO","WPRL","WICN","KBYU","WNCU","WVAS","WURC","KCSM","KWSO","WNYE","WSNC","WMOT","KCIE","KEXP","KGPR","KTEP","KUSC","WCPE","WEMC","WPPB","WVUB","WZPE","KDSC","KMST","KPSC","KQSC","KUAC","WCWP","WJSU","WSIE"};

public static class Listenable {
@@ -137,8 +138,7 @@

public String getCallLettersInName() {
// try to find call letters from title
- Pattern pattern = Pattern.compile("[KW][A-Z][A-Z][A-Z]");
- Matcher matcher = pattern.matcher(getName());
+ Matcher matcher = callLettersPattern.matcher(getName());
if (matcher.find())
return matcher.group();

=======================================
--- /Npr/src/org/npr/api/Story.java Fri Jan 6 11:10:49 2012
+++ /Npr/src/org/npr/api/Story.java Fri Jan 6 15:16:14 2012
@@ -285,9 +285,7 @@
private final String id;
private final String type;
private final boolean isPrimary;
- @SuppressWarnings("unused")
private final String title;
- @SuppressWarnings("unused")
private final String htmlLink;
private final String apiLink;

@@ -861,8 +859,7 @@
}

try {
- src = src.replaceAll("&s=[0-9]+", "");
- src += "&s=13";
+ src = src.replaceAll("&s=[0-9]+", "&s=13");
} catch (NullPointerException e) {
Log.e(LOG_TAG, "Error replacing size in story image parsing");
}

[Jeremy Wadsack]

One more change I'd suggest: 

+      String message = "Unknown error occurred.";

Should be internationalized.

--

Jeremy Wadsack

2012/1/6 <npr-and...@googlecode.com>