Refactoring based on code review feedback.

This commit is contained in:
Paul Hawke 2017-11-25 14:23:50 -06:00
parent 6d77243329
commit 07577d73a1
2 changed files with 13 additions and 17 deletions

View file

@ -16,6 +16,7 @@ import java.util.Map;
import java.util.regex.Matcher; import java.util.regex.Matcher;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import javax.inject.Inject;
import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException; import javax.xml.parsers.ParserConfigurationException;
@ -34,34 +35,26 @@ import timber.log.Timber;
public class MediaDataExtractor { public class MediaDataExtractor {
private final MediaWikiApi mediaWikiApi; private final MediaWikiApi mediaWikiApi;
private boolean fetched; private boolean fetched;
private String filename;
private ArrayList<String> categories; private ArrayList<String> categories;
private Map<String, String> descriptions; private Map<String, String> descriptions;
private String license; private String license;
private @Nullable LatLng coordinates; private @Nullable LatLng coordinates;
private LicenseList licenseList;
/** @Inject
* @param mwApi instance of MediaWikiApi public MediaDataExtractor(MediaWikiApi mwApi) {
* @param filename of the target media object, should include 'File:' prefix
*/
public MediaDataExtractor(String filename, LicenseList licenseList, MediaWikiApi mwApi) {
this.filename = filename;
this.categories = new ArrayList<>(); this.categories = new ArrayList<>();
this.descriptions = new HashMap<>(); this.descriptions = new HashMap<>();
this.fetched = false; this.fetched = false;
this.licenseList = licenseList;
this.mediaWikiApi = mwApi; this.mediaWikiApi = mwApi;
} }
/** /*
* Actually fetch the data over the network. * Actually fetch the data over the network.
* todo: use local caching? * todo: use local caching?
* *
* Warning: synchronous i/o, call on a background thread * Warning: synchronous i/o, call on a background thread
*/ */
public void fetch() throws IOException { public void fetch(String filename, LicenseList licenseList) throws IOException {
if (fetched) { if (fetched) {
throw new IllegalStateException("Tried to call MediaDataExtractor.fetch() again."); throw new IllegalStateException("Tried to call MediaDataExtractor.fetch() again.");
} }
@ -72,7 +65,7 @@ public class MediaDataExtractor {
extractCategories(result.getWikiSource()); extractCategories(result.getWikiSource());
// Description template info is extracted from preprocessor XML // Description template info is extracted from preprocessor XML
processWikiParseTree(result.getParseTreeXmlSource()); processWikiParseTree(result.getParseTreeXmlSource(), licenseList);
fetched = true; fetched = true;
} }
@ -91,7 +84,7 @@ public class MediaDataExtractor {
} }
} }
private void processWikiParseTree(String source) throws IOException { private void processWikiParseTree(String source, LicenseList licenseList) throws IOException {
Document doc; Document doc;
try { try {
DocumentBuilder docBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); DocumentBuilder docBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder();

View file

@ -22,6 +22,7 @@ import java.util.Date;
import java.util.Locale; import java.util.Locale;
import javax.inject.Inject; import javax.inject.Inject;
import javax.inject.Provider;
import dagger.android.support.DaggerFragment; import dagger.android.support.DaggerFragment;
import fr.free.nrw.commons.License; import fr.free.nrw.commons.License;
@ -56,7 +57,9 @@ public class MediaDetailFragment extends DaggerFragment {
return mf; return mf;
} }
@Inject MediaWikiApi mwApi; @Inject
Provider<MediaDataExtractor> mediaDataExtractorProvider;
private MediaWikiImageView image; private MediaWikiImageView image;
private MediaDetailSpacer spacer; private MediaDetailSpacer spacer;
private int initialListTop = 0; private int initialListTop = 0;
@ -192,13 +195,13 @@ public class MediaDetailFragment extends DaggerFragment {
@Override @Override
protected void onPreExecute() { protected void onPreExecute() {
extractor = new MediaDataExtractor(media.getFilename(), licenseList, mwApi); extractor = mediaDataExtractorProvider.get();
} }
@Override @Override
protected Boolean doInBackground(Void... voids) { protected Boolean doInBackground(Void... voids) {
try { try {
extractor.fetch(); extractor.fetch(media.getFilename(), licenseList);
return Boolean.TRUE; return Boolean.TRUE;
} catch (IOException e) { } catch (IOException e) {
Timber.d(e); Timber.d(e);