Drawing Routes on a Map

I have this class:

public class MapController
{
    private Polyline _currentPolyline; // The line connecting two locations on the map
    private GoogleMap _map;
    private Context _context;

    public MapController(GoogleMap map, Context context)
    {
        _map = map;
        _context = context;
    }

    /**
     * Draws a route on the map.
     *
     * @param directions the Google Map Directions JSON string with the routes
     * @throws JSONException
     */
    public void drawRoute(String directions) throws JSONException
    {
        // First parse the JSON to extract the points into an easily manipulated form
        List>> routes = new DirectionsHelper().parsePathsFromJson(directions);

        // Only one polyline can be shown on the map at a time
        hideRoute();

        ArrayList points;
        PolylineOptions polyLineOptions = null;

        // Traversing through routes
        for (int i = 0; i ();
            polyLineOptions = new PolylineOptions();
            List> path = routes.get(i);

            for (int j = 0; j  point = path.get(j);

                double lat = Double.parseDouble(point.get("lat"));
                double lng = Double.parseDouble(point.get("lng"));
                LatLng position = new LatLng(lat, lng);

                points.add(position);
            }

            polyLineOptions.addAll(points);
            polyLineOptions.color(ContextCompat.getColor(_context, R.color.colorPrimary));
        }

        _currentPolyline =  _map.addPolyline(polyLineOptions);
    }

    /**
     * Removes the currently drawn {@link Polyline} from the map.
     */
    public void hideRoute()
    {
        if (_currentPolyline != null)
        {
            _currentPolyline.removeRoute();
        }
    }
}

Is it generally a good practice to call hideRoute() inside the drawRoute() method or does it depend on other factors? For example: should the caller be responsible for clearing the map before drawing the new path

From an algorithm flow viewpoint, it looks good. However, I'd make a few small changes:

public void drawRoute(String directions) throws JSONException
{
    // First parse the JSON to extract the points into an easily manipulated form
    List>> routes = new DirectionsHelper().parsePathsFromJson(directions);

    // Only one polyline can be shown on the map at a time
    hideRoute();

    ArrayList points;
    PolylineOptions polyLineOptions = null;

    // Traversing through routes
    for (int i = 0; i ();
        polyLineOptions = new PolylineOptions();
        List> path = routes.get(i);

        for (int j = 0; j  point = path.get(j);

            double lat = Double.parseDouble(point.get("lat"));
            double lng = Double.parseDouble(point.get("lng"));
            LatLng position = new LatLng(lat, lng);

            points.add(position);
        }

        polyLineOptions.addAll(points);
        polyLineOptions.color(ContextCompat.getColor(_context, R.color.colorPrimary));
    }

    _currentPolyline =  _map.addPolyline(polyLineOptions);
}

In drawRoute, you first declare polyLineOptions = new PolylineOptions();, and then after the for loop, you assign values.

I think it'd be better if you moved the creation of PolylineOptions so that it's created and used in one place:

public void drawRoute(String directions) throws JSONException
{
    // First parse the JSON to extract the points into an easily manipulated form
    List>> routes = new DirectionsHelper().parsePathsFromJson(directions);

    // Only one polyline can be shown on the map at a time
    hideRoute();

    ArrayList points;
    PolylineOptions polyLineOptions = null;

    // Traversing through routes
    for (int i = 0; i ();
        List> path = routes.get(i);

        for (int j = 0; j  point = path.get(j);

            double lat = Double.parseDouble(point.get("lat"));
            double lng = Double.parseDouble(point.get("lng"));
            LatLng position = new LatLng(lat, lng);

            points.add(position);
        }

        polyLineOptions = new PolylineOptions();
        polyLineOptions.addAll(points);
        polyLineOptions.color(ContextCompat.getColor(_context, R.color.colorPrimary));
    }

    _currentPolyline =  _map.addPolyline(polyLineOptions);
}

And for the hideRoute function,

public void hideRoute()
{
    if (_currentPolyline != null)
    {
        _currentPolyline.removeRoute();
    }
}

You should set _currentPolyline to null so you can't remove it twice (if that matters), plus it allows the GC to clean up the polyline.

StackExchange Code Review Q#123156, answer score: 2