Skip to content

Commit

Permalink
multiple optimizations
Browse files Browse the repository at this point in the history 
- display copyright notice below song (if available)
- if no scheme is given in the URL, first try http:// and then https:// as prefix
- after downloading: first try to also deserialize the songs, then store them locally
- remember scroll position in song list view
- delete unused methods
- refactor anonymous implementations to lambdas
  • Loading branch information
@mathisdt
mathisdt committed Jan 5, 2019
1 parent 1fcd919 commit 8747dd7
Show file tree
Hide file tree
Showing 5 changed files with 156 additions and 105 deletions.
62 changes: 37 additions & 25 deletions app/src/main/java/org/zephyrsoft/sdbviewer/SettingsActivity.java
View file
Expand Up @@ -15,6 +15,9 @@
import android.support.v7.app.ActionBar;
import android.view.MenuItem;

import org.zephyrsoft.sdbviewer.fetch.SDBFetcher;
import org.zephyrsoft.sdbviewer.registry.Registry;

import java.util.List;

/**
Expand All @@ -34,30 +37,27 @@ public class SettingsActivity extends AppCompatPreferenceActivity {
* A preference value change listener that updates the preference's summary
* to reflect its new value.
*/
private static Preference.OnPreferenceChangeListener sBindPreferenceSummaryToValueListener = new Preference.OnPreferenceChangeListener() {
@Override
public boolean onPreferenceChange(Preference preference, Object value) {
String stringValue = value.toString();

if (preference instanceof ListPreference) {
// For list preferences, look up the correct display value in
// the preference's 'entries' list.
ListPreference listPreference = (ListPreference) preference;
int index = listPreference.findIndexOfValue(stringValue);

// Set the summary to reflect the new value.
preference.setSummary(
index >= 0
? listPreference.getEntries()[index]
: null);

} else {
// For all other preferences, set the summary to the value's
// simple string representation.
preference.setSummary(stringValue);
}
return true;
private static Preference.OnPreferenceChangeListener sBindPreferenceSummaryToValueListener = (preference, value) -> {
String stringValue = value.toString();

if (preference instanceof ListPreference) {
// For list preferences, look up the correct display value in
// the preference's 'entries' list.
ListPreference listPreference = (ListPreference) preference;
int index = listPreference.findIndexOfValue(stringValue);

// Set the summary to reflect the new value.
preference.setSummary(
index >= 0
? listPreference.getEntries()[index]
: null);

} else {
// For all other preferences, set the summary to the value's
// simple string representation.
preference.setSummary(stringValue);
}
return true;
};

/**
Expand Down Expand Up @@ -111,9 +111,15 @@ private void setupActionBar() {
public boolean onMenuItemSelected(int featureId, MenuItem item) {
int id = item.getItemId();
if (id == android.R.id.home) {
if (!super.onMenuItemSelected(featureId, item)) {
NavUtils.navigateUpFromSameTask(this);
Intent upIntent = NavUtils.getParentActivityIntent(this);
if (upIntent == null || NavUtils.shouldUpRecreateTask(this, upIntent)) {
// this activity is NOT part of this app's task, so create a new task when navigating up
navigateUpTo(upIntent);
} else {
// this activity is part of this app's task
finish();
}

return true;
}
return super.onMenuItemSelected(featureId, item);
Expand Down Expand Up @@ -164,6 +170,12 @@ public void onCreate(Bundle savedInstanceState) {
// guidelines.
bindPreferenceSummaryToValue(findPreference(getActivity().getString(R.string.pref_songs_url)));
bindPreferenceSummaryToValue(findPreference(getActivity().getString(R.string.pref_songs_reload_interval)));

findPreference(getActivity().getString(R.string.pref_songs_url))
.setOnPreferenceChangeListener((preference, newValue) -> {
Registry.get(SDBFetcher.class).invalidateSavedSongs(getActivity());
return true;
});
}

@Override
Expand Down
28 changes: 21 additions & 7 deletions app/src/main/java/org/zephyrsoft/sdbviewer/SongDetailFragment.java
View file
Expand Up @@ -61,10 +61,10 @@ public void onCreate(Bundle savedInstanceState) {
}
}

private boolean getBooleanPreference(String key, boolean defaultValue) {
private boolean getBooleanPreference(String key) {
SharedPreferences sharedPreferences =
PreferenceManager.getDefaultSharedPreferences(getContext());
return sharedPreferences.getBoolean(key, defaultValue);
return sharedPreferences.getBoolean(key, true);
}

@Override
Expand All @@ -73,12 +73,14 @@ public View onCreateView(@NonNull LayoutInflater inflater, ViewGroup container,
View rootView = inflater.inflate(R.layout.song_detail, container, false);

if (song != null) {
boolean showTranslation = getBooleanPreference(inflater.getContext().getString(R.string.pref_show_translation), true);
boolean showChords = getBooleanPreference(inflater.getContext().getString(R.string.pref_show_chords), true);
boolean showTranslation = getBooleanPreference(inflater.getContext().getString(R.string.pref_show_translation));
boolean showChords = getBooleanPreference(inflater.getContext().getString(R.string.pref_show_chords));

List<SongParser.SongElement> parsedLyrics = SongParser.parseLyrics(song, showChords, showTranslation);
List<SongParser.SongElement> parsedCopyright = SongParser.parseCopyright(song);

List<SongParser.SongElement> parsedSong = SongParser.parseLyrics(song, showChords, showTranslation);
boolean parsedSongContainsChords = false;
for (SongParser.SongElement element : parsedSong) {
for (SongParser.SongElement element : parsedLyrics) {
if (element.getType() == SongParser.SongElementEnum.CHORDS) {
parsedSongContainsChords = true;
break;
Expand All @@ -87,7 +89,7 @@ public View onCreateView(@NonNull LayoutInflater inflater, ViewGroup container,

SpannableStringBuilder formatted = new SpannableStringBuilder();

for (SongParser.SongElement element : parsedSong) {
for (SongParser.SongElement element : parsedLyrics) {
int start = formatted.length();
formatted.append(element.getElement());
int end = formatted.length();
Expand All @@ -103,6 +105,18 @@ public View onCreateView(@NonNull LayoutInflater inflater, ViewGroup container,
}
}

if (parsedCopyright.size() > 0) {
formatted.append("\n\n");
}
for (SongParser.SongElement element : parsedCopyright) {
int start = formatted.length();
formatted.append("\n").append(element.getElement());
int end = formatted.length();

formatted.setSpan(new RelativeSizeSpan(0.65f), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
formatted.setSpan(new StyleSpan(Typeface.ITALIC), start, end, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE);
}

((TextView) rootView.findViewById(R.id.song_detail)).setText(formatted);
}

Expand Down
103 changes: 73 additions & 30 deletions app/src/main/java/org/zephyrsoft/sdbviewer/SongListActivity.java
View file
Expand Up @@ -10,6 +10,7 @@
import android.preference.PreferenceManager;
import android.support.annotation.NonNull;
import android.support.v7.app.AppCompatActivity;
import android.support.v7.widget.LinearLayoutManager;
import android.support.v7.widget.RecyclerView;
import android.support.v7.widget.Toolbar;
import android.util.Log;
Expand Down Expand Up @@ -40,13 +41,30 @@
public class SongListActivity extends AppCompatActivity {

/**
* Whether or not the activity is in two-pane mode, i.e. running on a tablet
* device.
* Whether or not the activity is in two-pane mode, i.e. running on a tablet device.
*/
private boolean mTwoPane;

/**
* The topmost item's index in the song list.
* Used to keep the scroll position after navigation or data reloads.
*/
private int firstVisiblePosition = 0;

private SDBFetcher fetcher;

@Override
protected void onPause() {
RecyclerView recyclerView = findViewById(R.id.song_list);
assert recyclerView != null;

firstVisiblePosition =
((LinearLayoutManager) recyclerView.getLayoutManager()).findFirstCompletelyVisibleItemPosition();
Log.d(Constants.LOG_TAG, "saved first visible position " + firstVisiblePosition);

super.onPause();
}

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Expand All @@ -64,12 +82,19 @@ protected void onCreate(Bundle savedInstanceState) {
// activity should be in two-pane mode.
mTwoPane = true;
}
}

View recyclerView = findViewById(R.id.song_list);
@Override
protected void onResume() {
super.onResume();

RecyclerView recyclerView = findViewById(R.id.song_list);
assert recyclerView != null;
loadAndShow((RecyclerView) recyclerView);

loadAndShow(recyclerView);
}


@Override
public boolean onCreateOptionsMenu(Menu menu) {
// Inflate the menu; this adds items to the action bar if it is present.
Expand All @@ -82,7 +107,7 @@ public boolean onOptionsItemSelected(MenuItem item) {
switch (item.getItemId()) {
case R.id.action_refresh:
fetcher.invalidateSavedSongs(getApplicationContext());
loadAndShow((RecyclerView) findViewById(R.id.song_list));
loadAndShow(findViewById(R.id.song_list));
return true;

case R.id.action_settings:
Expand All @@ -102,33 +127,29 @@ public boolean onOptionsItemSelected(MenuItem item) {
private void loadAndShow(final @NonNull RecyclerView recyclerView) {
SharedPreferences sharedPreferences =
PreferenceManager.getDefaultSharedPreferences(getApplicationContext());
final String url = sharedPreferences.getString(getApplicationContext().getString(R.string.pref_songs_url), getString(R.string.pref_songs_url_default));
String url = sharedPreferences.getString(getApplicationContext().getString(R.string.pref_songs_url), getString(R.string.pref_songs_url_default));

findViewById(R.id.progressBar).setVisibility(View.VISIBLE);

Consumer<FetchSongsResult> onDone = new Consumer<FetchSongsResult>() {
@Override
public void accept(FetchSongsResult result) {
if (result.hasException()) {
Log.w(Constants.LOG_TAG, "could not load songs", result.getException());
Toast.makeText(getApplicationContext(), "Could not load songs. Is the URL \"" + url + "\" correct? If not, please go to Settings and edit it.", Toast.LENGTH_LONG).show();
} else {
recyclerView.setAdapter(new SimpleItemRecyclerViewAdapter(SongListActivity.this, result.getSongs(), mTwoPane));
}
findViewById(R.id.progressBar).setVisibility(View.INVISIBLE);
}
};
Runnable onAbort = new Runnable() {
@Override
public void run() {
findViewById(R.id.progressBar).setVisibility(View.INVISIBLE);
final String urlToUse = url;
Consumer<FetchSongsResult> onDone = result -> {
if (result.hasException()) {
Log.w(Constants.LOG_TAG, "could not load songs", result.getException());
Toast.makeText(getApplicationContext(), "Could not load songs. Is the URL \"" + urlToUse + "\" correct? If not, please go to Settings and edit it.", Toast.LENGTH_LONG).show();
} else {
recyclerView.setAdapter(new SimpleItemRecyclerViewAdapter(SongListActivity.this, result.getSongs(), mTwoPane));

recyclerView.getLayoutManager().scrollToPosition(firstVisiblePosition);
Log.d(Constants.LOG_TAG, "restored first visible position " + firstVisiblePosition);
}
findViewById(R.id.progressBar).setVisibility(View.INVISIBLE);
};
Runnable onAbort = () -> findViewById(R.id.progressBar).setVisibility(View.INVISIBLE);

try {
new FetchSongsTask(onDone, onAbort, url).execute();
new FetchSongsTask(onDone, onAbort, urlToUse).execute();
} catch (Exception e) {
onDone.accept(new FetchSongsResult(e));
onDone.accept(new FetchSongsResult(e, urlToUse));
}
}

Expand All @@ -145,14 +166,26 @@ private class FetchSongsTask extends AsyncTask<Void, Integer, FetchSongsResult>
}

protected FetchSongsResult doInBackground(Void... nothing) {
List<Song> songs;
try {
songs = fetcher.fetchSongs(getApplicationContext(), url);
if (!url.startsWith("http://") && !url.startsWith("https://")) {
try {
List<Song> songs = fetcher.fetchSongs(getApplicationContext(), "http://" + url);
return new FetchSongsResult(songs);
} catch (Exception ex) {
Log.w(Constants.LOG_TAG, "unsuccessfully tried URL \"" + url + "\" with http", ex);
}
try {
List<Song> songs = fetcher.fetchSongs(getApplicationContext(), "https://" + url);
return new FetchSongsResult(songs);
} catch (Exception ex) {
Log.w(Constants.LOG_TAG, "unsuccessfully tried URL \"" + url + "\" with https", ex);
}
}
List<Song> songs = fetcher.fetchSongs(getApplicationContext(), "http://" + url);
return new FetchSongsResult(songs);
} catch (Exception e) {
return(new FetchSongsResult(e));
return(new FetchSongsResult(e, url));
}

return new FetchSongsResult(songs);
}

@Override
Expand All @@ -168,13 +201,15 @@ protected void onPostExecute(FetchSongsResult result) {
private static class FetchSongsResult {
private List<Song> songs;
private Exception exception;
private String usedUrl;

FetchSongsResult(List<Song> songs) {
this.songs = songs;
}

FetchSongsResult(Exception exception) {
FetchSongsResult(Exception exception, String usedUrl) {
this.exception = exception;
this.usedUrl = usedUrl;
}

boolean hasException() {
Expand All @@ -188,6 +223,14 @@ List<Song> getSongs() {
Exception getException() {
return exception;
}

public String getUsedUrl() {
return usedUrl;
}

public void setUsedUrl(String usedUrl) {
this.usedUrl = usedUrl;
}
}

public static class SimpleItemRecyclerViewAdapter
Expand Down

0 comments on commit 8747dd7

Please sign in to comment.