Skip to content

Commit

Permalink
Merge branch 'misc_code_smells'
Browse files Browse the repository at this point in the history
  • Loading branch information
@simonpoole
simonpoole committed Nov 2, 2023
2 parents fad6d98 + c909186 commit cbaf37d
Show file tree
Hide file tree
Showing 8 changed files with 206 additions and 277 deletions.
2 changes: 0 additions & 2 deletions src/main/java/de/blau/android/Feedback.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package de.blau.android;

import java.io.Serializable;
import java.io.UnsupportedEncodingException;
import java.net.URLEncoder;
import java.util.HashMap;
Expand Down Expand Up @@ -39,7 +38,6 @@
import de.blau.android.resources.KeyDatabaseHelper.EntryType;
import de.blau.android.util.ActivityResultHandler;
import de.blau.android.util.ExecutorTask;
import de.blau.android.util.ScreenMessage;
import de.blau.android.util.Util;

/**
Expand Down
171 changes: 89 additions & 82 deletions src/main/java/de/blau/android/Logic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1970,25 +1970,6 @@ private void handleDelegatorException(@Nullable final FragmentActivity activity,
}
}

/**
* Splits all ways containing the node will be split at the nodes position
*
* @param activity activity this method was called from, if null no warnings will be displayed
* @param node node to split at
* @deprecated Only used for testing
*/
@Deprecated
public synchronized void performSplit(@Nullable final FragmentActivity activity, @NonNull final Node node) {
try {
createCheckpoint(activity, R.string.undo_action_split_ways);
displayAttachedObjectWarning(activity, node); // needs to be done before split
getDelegator().splitAtNode(node);
invalidateMap();
} catch (OsmIllegalOperationException | StorageException ex) {
handleDelegatorException(activity, ex);
}
}

/**
* Splits a way at a given node
*
Expand Down Expand Up @@ -3008,9 +2989,6 @@ public AsyncResult download(@NonNull final Context ctx, @NonNull Server server,
result = new AsyncResult(ErrorCodes.INVALID_DATA_RECEIVED, e.getMessage());
}
} catch (ParserConfigurationException | UnsupportedFormatException e) {
// crash and burn
// TODO this seems to happen when the API call returns text from a proxy or similar intermediate
// network device... need to display what we actually got
result = new AsyncResult(ErrorCodes.INVALID_DATA_RECEIVED, e.getMessage());
} catch (OsmServerException e) {
int code = e.getErrorCode();
Expand Down Expand Up @@ -3264,9 +3242,6 @@ private int downloadElement(@Nullable final Context ctx, @NonNull final String t
result = ErrorCodes.INVALID_DATA_RECEIVED;
}
} catch (ParserConfigurationException e) {
// crash and burn
// TODO this seems to happen when the API call returns text from a proxy or similar intermediate
// network device... need to display what we actually got
Log.e(DEBUG_TAG, "downloadElement problem with parser", e);
result = ErrorCodes.INVALID_DATA_RECEIVED;
} catch (OsmServerException e) {
Expand Down Expand Up @@ -3361,14 +3336,12 @@ protected AsyncResult doInBackground(Void arg) {
}
}
}
try {
// FIXME need to check if providing a handler makes sense here
if (!getDelegator().mergeData(osmParser.getStorage(), null)) {
result = new AsyncResult(ErrorCodes.DATA_CONFLICT);
}
} catch (IllegalStateException iex) {
result = new AsyncResult(ErrorCodes.CORRUPTED_DATA);

if (!getDelegator().mergeData(osmParser.getStorage(), null)) {
result = new AsyncResult(ErrorCodes.DATA_CONFLICT);
}
} catch (IllegalStateException iex) {
result = new AsyncResult(ErrorCodes.CORRUPTED_DATA);
} catch (SAXException e) {
Log.e(DEBUG_TAG, "downloadElement problem parsing", e);
Exception ce = e.getException();
Expand All @@ -3378,9 +3351,6 @@ protected AsyncResult doInBackground(Void arg) {
result = new AsyncResult(ErrorCodes.INVALID_DATA_RECEIVED);
}
} catch (ParserConfigurationException e) {
// crash and burn
// TODO this seems to happen when the API call returns text from a proxy or similar intermediate
// network device... need to display what we actually got
Log.e(DEBUG_TAG, "downloadElements problem parsing", e);
result = new AsyncResult(ErrorCodes.INVALID_DATA_RECEIVED);
} catch (OsmServerException e) {
Expand Down Expand Up @@ -3517,14 +3487,15 @@ protected AsyncResult doInBackground(Boolean arg) {
}
}
} catch (SAXException e) {
Log.e(DEBUG_TAG, "Problem parsing", e);
Log.e(DEBUG_TAG, "Problem parsing ", e);
Exception ce = e.getException();
if ((ce instanceof StorageException) && ((StorageException) ce).getCode() == StorageException.OOM) {
if (ce instanceof StorageException) {
return new AsyncResult(ErrorCodes.OUT_OF_MEMORY, ce.getMessage());
}
return new AsyncResult(ErrorCodes.INVALID_DATA_READ, e.getMessage());
} catch (StorageException sex) {
return new AsyncResult(ErrorCodes.OUT_OF_MEMORY, sex.getMessage());
} catch (ParserConfigurationException e) {
// crash and burn
Log.e(DEBUG_TAG, "Problem parsing", e);
return new AsyncResult(ErrorCodes.INVALID_DATA_READ, e.getMessage());
} catch (IOException e) {
Expand Down Expand Up @@ -3686,6 +3657,9 @@ protected AsyncResult doInBackground(Boolean arg) {
} finally {
SavingHelper.close(is);
}
} catch (StorageException sex) {
Log.e(DEBUG_TAG, "Problem reading PBF " + sex.getMessage());
return new AsyncResult(ErrorCodes.OUT_OF_MEMORY, sex.getMessage());
} catch (IOException | RuntimeException e) {
Log.e(DEBUG_TAG, "Problem parsing PBF ", e);
return new AsyncResult(ErrorCodes.INVALID_DATA_READ, e.getMessage());
Expand Down Expand Up @@ -3987,15 +3961,16 @@ protected void onPreExecute() {
protected Integer doInBackground(Void v) {
boolean result = true;
for (MapViewLayer layer : map.getLayers()) {
if (layer != null) {
try {
boolean layerResult = layer.onRestoreState(activity);
result = result && layerResult;
} catch (Exception e) {
// Never crash
Log.e(DEBUG_TAG, "loadLayerState failed for " + layer.getName() + " " + e.getMessage());
result = false;
}
if (layer == null) {
continue;
}
try {
boolean layerResult = layer.onRestoreState(activity);
result = result && layerResult;
} catch (Exception e) {
// Never crash
Log.e(DEBUG_TAG, "loadLayerState failed for " + layer.getName() + " " + e.getMessage());
result = false;
}
}
return result ? READ_OK : READ_FAILED;
Expand All @@ -4006,7 +3981,6 @@ protected void onPostExecute(Integer result) {
Log.d(DEBUG_TAG, "loadLayerState onPostExecute");
if (result != READ_FAILED) {
Log.d(DEBUG_TAG, "loadLayerState: state loaded correctly");
// FIXME if no bbox exists from data, try to use one from bugs
if (postLoad != null) {
postLoad.onSuccess();
}
Expand Down Expand Up @@ -4235,7 +4209,7 @@ protected Integer doInBackground(Void params) {
OsmGpxApi.uploadTrack(server, track, description, tags, visibility);
} catch (final OsmServerException e) {
Log.e(DEBUG_TAG, e.getMessage());
switch (e.getErrorCode()) { // FIXME use the same mechanics as for data upload
switch (e.getErrorCode()) {
case HttpURLConnection.HTTP_FORBIDDEN:
case HttpURLConnection.HTTP_UNAUTHORIZED:
result = ErrorCodes.INVALID_LOGIN;
Expand Down Expand Up @@ -5483,7 +5457,7 @@ public boolean clipboardIsEmpty() {
* @param activity this method was called from, if null no warnings will be displayed
* @param way way to circulize
*/
public void performCirculize(@Nullable FragmentActivity activity, Way way) {
public void performCirculize(@Nullable FragmentActivity activity, @NonNull Way way) {
if (way.getNodes().size() < 3) {
return;
}
Expand Down Expand Up @@ -5811,43 +5785,76 @@ private <T extends OsmElement> void displayAttachedObjectWarning(@Nullable Fragm
* @param checkRelationsOnly if true only check Relations
*/
private <T extends OsmElement> void displayAttachedObjectWarning(@Nullable FragmentActivity activity, Collection<T> list, boolean checkRelationsOnly) {
if (getFilter() != null && activity != null && showAttachedObjectWarning()) {
elementLoop: for (T e : list) {
if (!checkRelationsOnly) {
if (e instanceof Node) {
List<Way> ways = getWaysForNode((Node) e);
if (!ways.isEmpty()) {
for (Way w : ways) {
if (!getFilter().include(w, false)) {
AttachedObjectWarning.showDialog(activity);
break elementLoop;
}
}
}
} else if (e instanceof Way) {
for (Node n : ((Way) e).getNodes()) {
List<Way> ways = getWaysForNode(n);
if (!ways.isEmpty()) {
for (Way w : ways) {
if (!getFilter().include(w, false)) {
AttachedObjectWarning.showDialog(activity);
break elementLoop;
}
}
}
}
}
if (getFilter() == null || !showAttachedObjectWarning()) {
return;
}
for (T e : list) {
if (!checkRelationsOnly && ((e instanceof Node && displayAttachedObjectWarning(activity, (Node) e))
|| (e instanceof Way && displayAttachedObjectWarning(activity, (Way) e)))) {
continue;
}
displayAttachedRelationWarning(activity, e);
}
}

/**
* Display a warning if a hidden parent relation would be modified
*
* @param activity the calling activity
* @param w the Way
* @return true if a warning is displayed
*/
private void displayAttachedRelationWarning(@Nullable FragmentActivity activity, @NonNull OsmElement e) {
final Filter f = getFilter();
if (activity != null && f != null && e.hasParentRelations()) {
for (Relation r : e.getParentRelations()) {
if (!f.include(r, false)) {
AttachedObjectWarning.showDialog(activity);
return;
}
if (e.hasParentRelations()) {
for (Relation r : e.getParentRelations()) {
if (!getFilter().include(r, false)) {
AttachedObjectWarning.showDialog(activity);
break elementLoop;
}
}
}
}

/**
* Display a warning if a hidden object would be modified
*
* @param activity the calling activity
* @param w the Way
* @return true if a warning is displayed
*/
private boolean displayAttachedObjectWarning(@Nullable FragmentActivity activity, @NonNull Way w) {
if (activity != null) {
for (Node n : w.getNodes()) {
if (displayAttachedObjectWarning(activity, n)) {
return true;
}
}
}
return false;
}

/**
* Display a warning if a hidden object would be modified
*
* @param activity the calling activity
* @param n the Node
* @return true if a warning is displayed
*/
private boolean displayAttachedObjectWarning(@Nullable FragmentActivity activity, @NonNull Node n) {
final Filter f = getFilter();
if (activity != null && f != null) {
List<Way> ways = getWaysForNode(n);
if (!ways.isEmpty()) {
for (Way w : ways) {
if (!f.include(w, false)) {
AttachedObjectWarning.showDialog(activity);
return true;
}
}
}
}
return false;
}

/**
Expand Down
Loading

0 comments on commit cbaf37d

Please sign in to comment.