Google Maps Directions API

Question

I'm looking for all kinds of feedback: style issues, missed corner cases, performance etc.

My code is regarding Google Maps Directions API that is a service that calculates directions between locations using an HTTP request.

overview_polyline contains a single points object that holds an encoded polyline representation of the route.

steps contains detailed directions for walking or driving steps in transit directions.

legs[] contains an array which contains information about a leg of the route, between two locations within the given route.

html_instructions contains formatted instructions for this step, presented as an HTML text string.

start_address contains the human-readable address (typically a street address) resulting from reverse geocoding the start_location of this leg.

end_address contains the human-readable address (typically a street address) from reverse geocoding the end_location of this leg.

import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.URL;
import java.util.LinkedList;
import java.util.List;


public class TF {

    private String url = "https://maps.googleapis.com/maps/api/directions/json?";
    private String mainUrl = url;

    private String origin = null;
    private String destination = null;

    private String key = "";
    private String keyMatrix = "";

    private String urlMatrix = "https://maps.googleapis.com/maps/api/distancematrix/json?";
    private String urlMatrixCopy = urlMatrix;

    private List<String> list = null;
    private long[][] times = null;
    private String mode;

    private LinkedList<String[]> html_instructions_list = new LinkedList<>();

    public void setOrigin(String origin){
        this.origin = origin;
    }

    public void setDestination(String destination){
        this.destination = destination;
    }

    public void setListMatrix(List<String> list){
        this.list = list;
    }

    public void setMode(String mode){
        this.mode = mode;
    }

    public long[][] fetchMatrixResult() {

        urlMatrix = urlMatrixCopy;
        urlMatrix += "origins=";

        int length = list.size();
        times = new long[length][length];
        for(int i = 0; i < length; i++) {
            for (int j = 0; j < length; j++) {
                times[i][j] = 0;
            }
        }

        if(length == 1) {
            urlMatrix += list.get(0);
        } else {
            for(int i = 0; i < length - 1; i++) {
                urlMatrix += list.get(i) + "|";
            }

            urlMatrix += list.get(length - 1);
        }
        urlMatrix += "&destinations=";

        if(length == 1) {
            urlMatrix += list.get(0);
        }
        else {
            for(int i = 0; i < length - 1; i++){
                urlMatrix += list.get(i) + "|";
            }
            urlMatrix += list.get(length - 1);
        }

        urlMatrix += "&mode=" + mode;
        urlMatrix += "&key=" + keyMatrix;
        System.out.println(urlMatrix);

        String json = retrieveURLData();
        System.out.println("JSON: ");
        System.out.println(json);
        String[][] matrix = buildTimeMatrix(json, length);
        for(String[] row: matrix){
            System.out.print("| ");
            for(String element: row) {
                System.out.print(element + " | ");
            }

            System.out.println();
        }

        System.out.println("\nTIME MATRIX\n");
        for(long[] row: times){
            System.out.print("| ");
            for(long time: row){
                System.out.print(time+" | ");
            }

            System.out.println();
        }
        System.out.println("\n\n");

        return times;
    }

    private String retrieveURLData() {

        try {

            int read;
            char[] chars = new char[1024];
            URL urlPath = new URL(urlMatrix);
            StringBuilder buffer = new StringBuilder();
            BufferedReader reader = new BufferedReader(new InputStreamReader(urlPath.openStream()));
            while ((read = reader.read(chars)) != -1) {
                buffer.append(chars, 0, read);
            }

            return buffer.toString();

        } catch (IOException ioe){
            ioe.printStackTrace();
        }

        return null;
    }

    private String[][] buildTimeMatrix(String json, int length){

        System.out.println("\n\n");
        String[][] matrix = new String[length + 1][length + 1];

        try{
            JSONObject jObj = new JSONObject(json);
            JSONArray jArray = (JSONArray) jObj.get("destination_addresses");
            for(int i = 1; i < length + 1; i++) {

                matrix[0][i] = jArray.getString(i - 1);
                matrix[i][0] = jArray.getString(i - 1);
            }

            jArray = (JSONArray) jObj.get("rows");
            for(int i = 0; i < jArray.length(); i++) {

                JSONObject row = (JSONObject)jArray.get(i);
                JSONArray elements = (JSONArray) row.get("elements");

                for(int j = 0; j < elements.length(); j++) {
                    String text;
                    long timeValue;

                    JSONObject element = (JSONObject) elements.get(j);
                    JSONObject duration = (JSONObject) element.get("duration");
                    text = (String) duration.get("text");
                    timeValue = duration.getLong("value");

                    matrix[i+1][j+1] = text;
                    times[i][j] = timeValue;
                }
            }
        } catch (JSONException je){
            je.printStackTrace();
        }

        return matrix;
    }

    public String[] getHtmlInstructions(int index){

        return html_instructions_list.get(index);
    }

    public String fetchResult() {

        url = mainUrl;
        url += "origin=" + origin;
        url += "&destination=" + destination;
        url += "&mode=" + mode;
        url += "&key=" + key;

        try{

            System.out.println(url);

            int read;
            URL urlPath = new URL(url);
            char[] chars = new char[1024];
            StringBuilder buffer = new StringBuilder();
            BufferedReader reader = new BufferedReader(new InputStreamReader(urlPath.openStream()));
            while ((read = reader.read(chars)) != -1) {
                buffer.append(chars, 0, read);
            }

            JSONObject json = new JSONObject(buffer.toString());
            System.out.println(buffer.toString());

            JSONArray array = (JSONArray) json.get("routes");
            if(array.length() == 0){
                return "NO ROUTE";
            }

            JSONObject obj = (JSONObject) array.get(0);

            JSONArray array1 = (JSONArray) obj.get("legs");
            JSONObject obj2 = (JSONObject) array1.get(0);
            String origin = obj2.getString("start_address");
            String destination = obj2.getString("end_address");
            String header = origin +" --> "+destination+"\n";
            JSONArray array2 = (JSONArray) obj2.get("steps");
            String[] html_instructions = new String[array2.length()+1];
            html_instructions[0] = header;
            for(int i = 0; i < array2.length(); i++){
                JSONObject obj2a = (JSONObject) array2.get(i);
                String html = obj2a.getString("html_instructions");
                html_instructions[i+1] = html;
            }
            html_instructions_list.addLast(html_instructions);

            JSONObject obj3 = (JSONObject) obj.get("overview_polyline");
            return obj3.getString("points");

        }  catch(IOException | JSONException ex){

            ex.printStackTrace();
        }

        return null;
    }
}

Answer 1

    private long[][] times = null;

So this starts as unset. In the method fetchMatrixResult, you initialize it. You set values in it in buildTimeMatrix. Why? You never use those values. The only time that you read from times is to return it from fetchMatrixResult. But each time that you call fetchMatrixResult, you throw away the old times array and create a new one.

The only way that this pattern works is if you call buildTimeMatrix from fetchMatrixResult. Since you don't use times anywhere but those two methods, consider enforcing this relationship. Remove the object field times and replace it with a local variable in fetchMatrixResult. Replace it with a parameter in buildTimeMatrix.

That way, you don't have an object field hanging out for the life of the object even though you only use it when one method is called externally (buildTimeMatrix is only called internally). And the usage pattern is enforced rather than maintained by protocol.

                    String text;
                    long timeValue;

                    JSONObject element = (JSONObject) elements.get(j);
                    JSONObject duration = (JSONObject) element.get("duration");
                    text = (String) duration.get("text");
                    timeValue = duration.getLong("value");

                    matrix[i+1][j+1] = text;
                    times[i][j] = timeValue;

It seems like we could shorten this. For example:

                    JSONObject element = (JSONObject) elements.get(j);
                    JSONObject duration = (JSONObject) element.get("duration");
                    String text = (String) duration.get("text");
                    long timeValue = duration.getLong("value");

                    matrix[i+1][j+1] = text;
                    times[i][j] = timeValue;

But I think that I'd go ahead and say

                    JSONObject element = (JSONObject) elements.get(j);
                    JSONObject duration = (JSONObject) element.get("duration");

                    matrix[i+1][j+1] = (String) duration.get("text");
                    times[i][j] = duration.getLong("value");

There doesn't seem to be a compelling need for intermediate variables here.

The main argument in favor of the intermediate variables is that they act as comments. But I would think that a better solution would be to name matrix and times more descriptively.

    private String url = "https://maps.googleapis.com/maps/api/directions/json?";
    private String mainUrl = url;

This could be just

    private static final String MAIN_URL = "https://maps.googleapis.com/maps/api/directions/json?";

Then

        url = mainUrl;

could become

          String url = MAIN_URL;

Again, no need to carry around a field when a local variable will do.

        try {

            int read;
            char[] chars = new char[1024];
            URL urlPath = new URL(urlMatrix);
            StringBuilder buffer = new StringBuilder();
            BufferedReader reader = new BufferedReader(new InputStreamReader(urlPath.openStream()));
            while ((read = reader.read(chars)) != -1) {
                buffer.append(chars, 0, read);
            }

            return buffer.toString();

        } catch (IOException ioe){
            ioe.printStackTrace();
        }

You never close the reader. You could add an explicit close, but I would prefer to use the try-with-resources form:

        URL urlPath = new URL(urlMatrix);
        try (BufferedReader reader = new BufferedReader(new InputStreamReader(urlPath.openStream()))) {

            int read;
            char[] chars = new char[1024];
            StringBuilder buffer = new StringBuilder();
            while ((read = reader.read(chars)) != -1) {
                buffer.append(chars, 0, read);
            }

            return buffer.toString();

        } catch (IOException ioe){
            ioe.printStackTrace();
        }

This will handle a number of edge cases that you might miss if doing it manually.