Create and organize some classes (hierarchy)

Question

I'm trying to figure out what is the best way to create some objects in my application. The application is already running, but I want to structure things well and clean.

These objects represents paths and they are complicated (they receive various parameters in their constructors).

There are four types of classes that represents a particular path:

• Gpx
• Road
• TrackMarkers
• JoinedMarkers

In the application there can exist only one path (Gpx, TrackMarkers, JoinedMarkers) and (if the user wants to) a Road (Road class represents the road to the path). A Road can't exist alone.

My MainActivity must have access to the data of these classes once created. I did not create a hierarchy of these classes (or implemented a creational patterns like factory method, abstract factory, builder etc...) and these classes (which are similar) contains duplicated code (except for JoinedMarkers that is simpler).

All the code of these classes:

Gpx

public class Gpx {
 /** lunghezza del tracciato gpx */
 private float length;
 /** elevazione media del tracciato gpx */
 private double averageElevation;
 /** lista di coordinate del tracciato */
 private List<LatLng> coordinates;
 /** file relativo a questo percorso gpx */
 private File file;
 /** contesto dell'activity principale */
 private Context context;
 public Gpx(File file, Context context) {
 coordinates = new ArrayList<LatLng>();
 this.file = file;
 this.file = file;
 build();
 }
 /**
 * Esegue la costruzione del tracciato, inizializzando i campi privati
 */
 private void build() {
 ParserAsyncTask parserAsyncTask = new ParserAsyncTask();
 parserAsyncTask.execute(file);
 }
 /**
 * AsyncTask che effettua il parsing del file gpx
 */
 private class ParserAsyncTask extends AsyncTask<File,Void,Void> {
 private ProgressDialog mDialog;
 protected void onPreExecute() {
 mDialog = ProgressDialog.show(context, "", "Caricamento...", true);
 }
 protected void onPostExecute(Void unused) {
 if(mDialog != null) {
 if(mDialog.isShowing()) {
 mDialog.dismiss();
 }
 }
 //calcolo la lunghezza del tracciato
 calculatesLength();
 }
 protected Void doInBackground(File... files) {
 //PARSING ALGORITHM, OMITTED
 }
 }
 private void calculatesLength() {
 for(int i=0; i < coordinates.size() - 1; i++) {
 LatLng firstLatLng = coordinates.get(i);
 LatLng secondLatLng = coordinates.get(i+1);
 length += Utilities.getInstance().getDistance(firstLatLng,secondLatLng);
 }
 }
 public float getLength() {
 return length;
 }
 public double getAverageElevation() {
 return averageElevation;
 }
 public List<LatLng> getCoordinates() {
 return coordinates;
 }
}

JoinedMarkers

public class JoinedMarkers {
 /** lunghezza del tracciato */
 private float length;
 /** lista di markers */
 private List<Marker> markers;
 public JoinedMarkers(List<Marker> markers) {
 this.markers = markers;
 calculatesLength();
 }
 private void calculatesLength() {
 for(int i=0; i < markers.size() - 1; i++) {
 LatLng firstLatLng = markers.get(i).getPosition();
 LatLng secondLatLng = markers.get(i+1).getPosition();
 length += Utilities.getInstance().getDistance(firstLatLng,secondLatLng);
 }
 }
 public float getLength() {
 return length;
 }
}

TrackMarkers

public class TrackMarkers {
 /** lunghezza del tracciato */
 private float length;
 /** elevazione media */
 private double averageElevation;
 /** modalità di viaggio */
 private String travelMode;
 /** lista di coordinate del tracciato */
 private List<LatLng> coordinates;
 /** lista di markers per costruire il percorso */
 private List<Marker> markers;
 /** tempo di percorrenza stimato **/
 private int duration;
 /** contesto dell'Activity principale */
 private Context context;
 /** timeout per la connessione */
 private int connectionTimeout;
 /** timeout per il download */
 private int dataTimeout;
 /** flag per il calcolo della lunghezza */
 private boolean flag;
 public TrackMarkers(Context context, String travelMode, List<Marker> markers,
 int connectionTimeout, int dataTimeout) {
 flag = false;
 this.travelMode = travelMode;
 this.markers = markers;
 this.context = context;
 this.connectionTimeout = connectionTimeout;
 this.dataTimeout = dataTimeout;
 coordinates = new ArrayList<LatLng>();
 //avvia la costruzione del tracciato
 build();
 }
 private void build() {
 for(int i = 0; i < markers.size() - 1; i++) {
 Marker firstMarker = markers.get(i);
 Marker secondMarker = markers.get(i + 1);
 //I extract latitude and longitude from the markers
 LatLng firstLatLng = new LatLng(firstMarker.getPosition().latitude, firstMarker.getPosition().longitude);
 LatLng secondLatLng = new LatLng(secondMarker.getPosition().latitude, secondMarker.getPosition().longitude);
 //Ogni volta si costruisce un URL aggiornato per la nuova coppia di markers
 String URL = Utilities.getInstance().makeURL(firstLatLng.latitude, firstLatLng.longitude,
 secondLatLng.latitude, secondLatLng.longitude, travelMode);
 if(i == markers.size() - 2) {
 flag = true;
 }
 MultipleTracksAsyncTask multipleTracksAsyncTask = new MultipleTracksAsyncTask(URL);
 multipleTracksAsyncTask.execute();
 }
 }
 private class MultipleTracksAsyncTask extends AsyncTask<Void,Void,String> {
 private ProgressDialog mDialog;
 private String URL;
 public MultipleTracksAsyncTask(String URL) {
 this.URL = URL;
 }
 protected void onPreExecute() {
 mDialog = ProgressDialog.show(context, "", "Loading...", true);
 }
 protected void onPostExecute(String result) {
 if(mDialog != null) {
 if(mDialog.isShowing()) {
 mDialog.dismiss();
 }
 }
 //effettuo il parsing del JSON
 if(result != null) {
 parseJSON(result);
 //se la computazione è finita, calcola la lunghezza
 if(flag) {
 calculatesLength();
 }
 }
 }
 public String doInBackground(Void... param) {
 JSONRequest jParser = new JSONRequest();
 String json = jParser.getJSONFromUrl(URL,connectionTimeout,dataTimeout);
 return json;
 }
 }
 private void parseJSON(String result) {
 //PARSING ALGORITHM OMITTED
 }
 private void calculatesLength() {
 for(int i=0; i < coordinates.size() - 1; i++) {
 LatLng firstLatLng = coordinates.get(i);
 LatLng secondLatLng = coordinates.get(i+1);
 length += Utilities.getInstance().getDistance(firstLatLng,secondLatLng);
 }
 }
 public float getLength() {
 return length;
 }
 public double getAverageElevation() {
 return averageElevation;
 }
 public int getDuration() {
 return duration;
 }
 public List<LatLng> getCoordinates() {
 return coordinates;
 }
}

Road

public class Road {
 /** lista di coordinate */
 private List<LatLng> coordinates;
 /** lunghezza del percorso*/
 private float length;
 /** tempo di percorrenza stimato */
 private int duration;
 /** URL per la richiesta HTTP */
 private String URL;
 /** contesto dell'Activity principale */
 private Context context;
 /** timeout per la connessione */
 private int connectionTimeout;
 /** timeout per il download */
 private int dataTimeout;
 public Road(Context context, String URL, int connectionTimeout,
 int dataTimeout) {
 coordinates = new ArrayList<LatLng>();
 this.context = context;
 this.URL = URL;
 this.connectionTimeout = connectionTimeout;
 this.dataTimeout = dataTimeout;
 build();
 }
 private void build() {
 PathToTrackAsyncTask pathToTrackAsyncTask = new PathToTrackAsyncTask();
 pathToTrackAsyncTask.execute();
 }
 private class PathToTrackAsyncTask extends AsyncTask<Void, Void, String> {
 private ProgressDialog mDialog;
 protected void onPreExecute() {
 mDialog = ProgressDialog.show(context, "", "Caricamento...", true);
 }
 protected void onPostExecute(String result) {
 if(mDialog != null) {
 if(mDialog.isShowing()) {
 mDialog.dismiss();
 }
 }
 if(result != null) {
 parseJSON(result);
 }
 }
 @Override
 protected String doInBackground(Void... params) {
 //Istanzio il JSONRequest
 JSONRequest jParser = new JSONRequest();
 //Ottengo la stringa JSON
 String json = jParser.getJSONFromUrl(URL,connectionTimeout,dataTimeout);
 //Ritorno la stringa per l'onPostExecute
 return json;
 }
 private void parseJSON(String result) {
 //PARSING ALGORITHM OMITTED (very similar to the previous class)
 }
 }
 public float getLength() {
 return length;
 }
 public int getDuration() {
 return duration;
 }
 public List<LatLng> getCoordinates() {
 return coordinates;
 }
}

UPDATE

AbstractAsyncTask

public abstract class AbstractAsyncTask extends AsyncTask<Void,Void,String> {
 private ProgressDialog dialog;
 private String URL;
 private Context context;
 private int connectionTimeout;
 private int dataTimeout;
 private List<LatLng> coordinates;
 public AbstractAsyncTask(Context context, String URL,
 int connectionTimeout, int dataTimeout, List<LatLng> coordinates) {
 this.context = context;
 this.URL = URL;
 this.connectionTimeout = connectionTimeout;
 this.dataTimeout = dataTimeout;
 this.coordinates = coordinates;
 }
 protected void onPreExecute() {
 dialog = ProgressDialog.show(context, "", "Loading...", true);
 }
 protected void onPostExecute(String result) {
 if(dialog != null) {
 if(dialog.isShowing()) {
 dialog.dismiss();
 }
 }
 if(result != null) {
 parseJSON(result);
 }
 }
 @Override
 protected String doInBackground(Void... params) {
 JSONRequest jParser = new JSONRequest();
 String json = jParser.getJSONFromUrl(URL,connectionTimeout,dataTimeout);
 return json;
 }
 /**
 * JSON parsing 
 */
 protected void parseJSON(String result) {
 try {
 final JSONObject json = new JSONObject(result);
 JSONArray routeArray = json.getJSONArray("routes");
 JSONObject routes = routeArray.getJSONObject(0);
 JSONArray legsArray = routes.getJSONArray("legs");
 JSONObject legsObject = legsArray.getJSONObject(0);
 JSONObject distanceObject = legsObject.getJSONObject("distance");
 JSONObject durationObject = legsObject.getJSONObject("duration");
 JSONObject overviewPolylines = routes.getJSONObject("overview_polyline");
 String encodedString = overviewPolylines.getString("points");
 ArrayList<LatLng> res = Utilities.getInstance().decodePoly(encodedString);
 coordinates.addAll(res);
 //for Road
 //length = distanceObject.getInt("value");
 //duration = durationObject.getInt("value");
 //for TrackMarkers
 //length += distanceObject.getInt("value");
 //duration += durationObject.getInt("value");
 updateLenghtAndDuration(distanceObject,durationObject);
 }
 catch (JSONException e) {
 e.printStackTrace();
 Toast.makeText(context, "Path is not available", Toast.LENGTH_SHORT).show();
 }
 }
 protected abstract void updateLenghtAndDuration(JSONObject distanceObject, JSONObject durationObject);
}

RoadAsyncTask

public class RoadAsyncTask extends AbstractAsyncTask {
 public RoadAsyncTask(Context context, String URL,
 int connectionTimeout, int dataTimeout, List<LatLng> coordinates,
 float length, int duration) {
 super(context, URL, connectionTimeout, dataTimeout, coordinates,
 length, duration);
 }
 @Override
 protected void updateLenghtAndDuration(JSONObject distanceObject, JSONObject durationObject) {
 try {
 length = distanceObject.getInt("value");
 duration = durationObject.getInt("value");
 } catch (JSONException e) {
 e.printStackTrace();
 }
 }
}

TrackMarkersAsyncTask

public class TrackMarkersAsyncTask extends AbstractAsyncTask {
 public TrackMarkersAsyncTask(Context context, String URL, int connectionTimeout, int dataTimeout, List<LatLng> coordinates, float length, int duration) {
 super(context, URL, connectionTimeout, dataTimeout, coordinates, length, duration);
 }
 @Override
 protected void updateLenghtAndDuration(JSONObject distanceObject, JSONObject durationObject) {
 try {
 length += distanceObject.getInt("value");
 duration += durationObject.getInt("value");
 } catch (JSONException e) {
 e.printStackTrace();
 }
 }
}

• skiwi: I'm a bit sad to see the (wasted) time to create localized documentation. It is a good thing that you document, but they should really be in English.
• Loris: Can't you understand?
• 200_success: @skiwi Actually, the comments in Italian are pretty much redundant, and don't say much more than the English identifiers. That may be an ideal solution for a bilingual programming team: the code stays English, but there is some translation help for programmers who may understand Italian better.

Answer 1

Duplicated code

You have indeed some duplicated code, sometimes just a little off.
You can make abstract class for MultipleTracksAsyncTask and PathToTrackAsyncTask and move the duplicated code to there.

I should move to an private method of the abstract class.

if(mDialog != null) {
 if(mDialog.isShowing()) {
 mDialog.dismiss();
 }
}

Same for this but I should make an abstract method and make the implementation later cause it's for the 2 classes a little different :

if(result != null) {
 parseJSON(result);
}

So the result in the abstract class could be (You know what exactly the code does so make the names better for your cause):

@Override
protected void onPostExecute(String result) {
 updateMDialog();
 updateResult();
}

The same for your basic classes. I see lenght and context always popup(except for JoinedMarkers) and Road and TrackMarkers have even more in common.

Try abstracting this to an abstract class.

Dangerous situation

Now I guess you have overlooked it but you have a potential bug in your code.
In the class Gpx you have the List<LatLng> and calculates the distance in async task.
You don't provide a setter cause that will break your distance. The fault is that you have this getter.

public List<LatLng> getCoordinates() {
 return coordinates;
}

What's wrong with it?

Look at following code :

gpx.getCoordinates().clear();

Your list will be empty and your lenght will have incorrect value according this new list.
Program defensive and return an unmodifiable list back so no one can change it with the getter.

public List<LatLng> getCoordinates() {
 return Collections.unmodifiableList(coordinates);
}

Watch out: I put it here to point it out.
If after init the coordinates will not change anymore make it there unmodifiable.
So you won't constantly making a new unmodifiable list when you call the getter.

• Loris: Oooh thanks! You are very helpful. I have updated the question and i added the class AbstractAsyncTask. Can you check it out and tell me if it's ok?
• chillworld: looking good, just watch out if the protected void parseJson(String result) is the same code for all implementations, otherwise seperate the duplicated code from the specific implementation code. If you want that users can't override some methods, make the methods final.
• Loris: Yes the parsing is the same for the two classes. It's a bit different for the commented part, for which i created an abstract method. I updated again the question with the other two AsyncTask and i did some other little change too. It's right that the variables length and duration are protected? They are variables of the class (Road or TrackMarkers) that need to be updated by AsyncTask. However now the method onPostExecute() is the same for the two classes.
• chillworld: You are very correct with the protected. refactoring is good, abstracting also but don't go to abstract. If a method is in 2 implementations the same, and some other implementations not, so be it (certainly for 2 lines of code). If it's in all the implementations the same move it higher to remove the duplicated code.

Answer 2

You should not pass the Context to those classes (separation of concerns/ single responsibility principle). You could add the Context as a constructor parameter to the AsyncTask's; or maybe it would be better to define the AsyncTask's where the computation is launched (and the Context accessible) instead of as inner classes of your four classes.

I can't comment much about your four different classes since I lack domain knowledge and you did not explain much. However, they seem very similar so maybe you could get rid of some of them, or use inheritance.