Removed TrackPointFactory.

New TrackPoints are now always created and old ones not reused.

src/androidTest/java/de/dennisguse/opentracks/content/provider/CustomContentProviderUtilsTest.java

@@ -33,7 +33,6 @@ import org.mockito.junit.MockitoJUnitRunner;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicInteger;
 
 import de.dennisguse.opentracks.content.data.TestDataUtil;
 import de.dennisguse.opentracks.content.data.Track;
@@ -76,68 +75,43 @@ public class CustomContentProviderUtilsTest {
 
     @Test
     public void testLocationIterator_noPoints() {
-        testIterator(1, 0, 1, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
-    }
-
-    @Test
-    public void testLocationIterator_customFactory() {
-        final TrackPoint location = new TrackPoint(new Location("test_location"));
-        final AtomicInteger counter = new AtomicInteger();
-        testIterator(1, 15, 4, false, new TrackPointFactory() {
-            @Override
-            public TrackPoint create() {
-                counter.incrementAndGet();
-                return location;
-            }
-        });
-        // Make sure we were called exactly as many times as we had track points.
-        Assert.assertEquals(15, counter.get());
-    }
-
-    @Test
-    public void testLocationIterator_nullFactory() {
-        try {
-            testIterator(1, 15, 4, false, null);
-            Assert.fail("Expecting IllegalArgumentException");
-        } catch (IllegalArgumentException e) {
-            // Expected.
-        }
+        testIterator(1, 0, 1, false);
     }
 
     @Test
     public void testLocationIterator_noBatchAscending() {
-        testIterator(1, 50, 100, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
-        testIterator(2, 50, 50, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
+        testIterator(1, 50, 100, false);
+        testIterator(2, 50, 50, false);
     }
 
     @Test
     public void testLocationIterator_noBatchDescending() {
-        testIterator(1, 50, 100, true, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
-        testIterator(2, 50, 50, true, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
+        testIterator(1, 50, 100, true);
+        testIterator(2, 50, 50, true);
     }
 
     @Test
     public void testLocationIterator_batchAscending() {
-        testIterator(1, 50, 11, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
-        testIterator(2, 50, 25, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
+        testIterator(1, 50, 11, false);
+        testIterator(2, 50, 25, false);
     }
 
     @Test
     public void testLocationIterator_batchDescending() {
-        testIterator(1, 50, 11, true, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
-        testIterator(2, 50, 25, true, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
+        testIterator(1, 50, 11, true);
+        testIterator(2, 50, 25, true);
     }
 
     @Test
     public void testLocationIterator_largeTrack() {
-        testIterator(1, 20000, 2000, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
+        testIterator(1, 20000, 2000, false);
     }
 
-    private List<TrackPoint> testIterator(long trackId, int numPoints, int batchSize, boolean descending, TrackPointFactory trackPointFactory) {
+    private List<TrackPoint> testIterator(long trackId, int numPoints, int batchSize, boolean descending) {
         long lastPointId = initializeTrack(trackId, numPoints);
         contentProviderUtils.setDefaultCursorBatchSize(batchSize);
         List<TrackPoint> locations = new ArrayList<>(numPoints);
-        try (TrackPointIterator it = contentProviderUtils.getTrackPointLocationIterator(trackId, -1L, descending, trackPointFactory)) {
+        try (TrackPointIterator it = contentProviderUtils.getTrackPointLocationIterator(trackId, -1L, descending)) {
             while (it.hasNext()) {
                 TrackPoint loc = it.next();
                 Assert.assertNotNull(loc);
@@ -173,7 +147,7 @@ public class CustomContentProviderUtilsTest {
         // Load all inserted trackPoints.
         long lastPointId = -1;
         int counter = 0;
-        try (TrackPointIterator it = contentProviderUtils.getTrackPointLocationIterator(id, -1L, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY)) {
+        try (TrackPointIterator it = contentProviderUtils.getTrackPointLocationIterator(id, -1L, false)) {
             while (it.hasNext()) {
                 it.next();
                 lastPointId = it.getTrackPointId();
@@ -688,7 +662,7 @@ public class CustomContentProviderUtilsTest {
     }
 
     /**
-     * Tests the method {@link ContentProviderUtils#getTrackPointLocationIterator(long, long, boolean, TrackPointFactory)} in descending.
+     * Tests the method {@link ContentProviderUtils#getTrackPointLocationIterator(long, long, boolean)} in descending.
      */
     @Test
     public void testGetTrackPointLocationIterator_desc() {
@@ -704,7 +678,7 @@ public class CustomContentProviderUtilsTest {
 
         long startTrackPointId = trackpointIds[9];
 
-        TrackPointIterator trackPointIterator = contentProviderUtils.getTrackPointLocationIterator(trackId, startTrackPointId, true, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
+        TrackPointIterator trackPointIterator = contentProviderUtils.getTrackPointLocationIterator(trackId, startTrackPointId, true);
         for (int i = 0; i < trackpointIds.length; i++) {
             Assert.assertTrue(trackPointIterator.hasNext());
             TrackPoint trackPoint = trackPointIterator.next();
@@ -715,7 +689,7 @@ public class CustomContentProviderUtilsTest {
     }
 
     /**
-     * Tests the method {@link ContentProviderUtils#getTrackPointLocationIterator(long, long, boolean, TrackPointFactory)} in ascending.
+     * Tests the method {@link ContentProviderUtils#getTrackPointLocationIterator(long, long, boolean)} in ascending.
      */
     @Test
     public void testGetTrackPointLocationIterator_asc() {
@@ -731,7 +705,7 @@ public class CustomContentProviderUtilsTest {
 
         long startTrackPointId = trackpointIds[0];
 
-        TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(trackId, startTrackPointId, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY);
+        TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(trackId, startTrackPointId, false);
         for (int i = 0; i < trackpointIds.length; i++) {
             Assert.assertTrue(locationIterator.hasNext());
             TrackPoint trackPoint = locationIterator.next();

src/main/java/de/dennisguse/opentracks/content/TrackDataHub.java

@@ -34,7 +34,6 @@ import de.dennisguse.opentracks.content.data.Track;
 import de.dennisguse.opentracks.content.data.TrackPoint;
 import de.dennisguse.opentracks.content.data.Waypoint;
 import de.dennisguse.opentracks.content.provider.ContentProviderUtils;
-import de.dennisguse.opentracks.content.provider.TrackPointFactory;
 import de.dennisguse.opentracks.content.provider.TrackPointIterator;
 import de.dennisguse.opentracks.util.LocationUtils;
 import de.dennisguse.opentracks.util.PreferencesUtils;
@@ -392,7 +391,7 @@ public class TrackDataHub implements DataSourceListener, SharedPreferences.OnSha
         int samplingFrequency = -1;
         boolean includeNextPoint = false;
 
-        try (TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(selectedTrackId, localLastSeenLocationId + 1, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY)) {
+        try (TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(selectedTrackId, localLastSeenLocationId + 1, false)) {
 
             while (locationIterator.hasNext()) {
                 TrackPoint trackPoint = locationIterator.next();

src/main/java/de/dennisguse/opentracks/content/data/TrackPoint.java

@@ -185,9 +185,4 @@ public class TrackPoint {
     public float bearingTo(@NonNull Location dest) {
         return location.bearingTo(dest);
     }
-
-    public void reset() {
-        location.reset();
-        sensorDataSet = null;
-    }
 }

src/main/java/de/dennisguse/opentracks/content/provider/ContentProviderUtils.java

@@ -626,10 +626,9 @@ public class ContentProviderUtils {
      *
      * @param cursor   the cursor pointing to a trackPoint.
      * @param indexes  the cached track points indexes
-     * @param trackPoint the track point
      */
-    static void fillTrackPoint(Cursor cursor, CachedTrackPointsIndexes indexes, TrackPoint trackPoint) {
-        trackPoint.reset();
+    static TrackPoint fillTrackPoint(Cursor cursor, CachedTrackPointsIndexes indexes) {
+        TrackPoint trackPoint = new TrackPoint();
 
         if (!cursor.isNull(indexes.longitudeIndex)) {
             trackPoint.setLongitude(((double) cursor.getInt(indexes.longitudeIndex)) / 1E6);
@@ -658,6 +657,8 @@ public class ContentProviderUtils {
         float power = cursor.isNull(indexes.sensorPowerIndex) ? SensorDataSet.DATA_UNAVAILABLE : cursor.getFloat(indexes.sensorPowerIndex);
 
         trackPoint.setSensorDataSet(new SensorDataSet(heartRate, cadence, power, SensorDataSet.DATA_UNAVAILABLE, trackPoint.getTime()));
+
+        return trackPoint;
     }
 
     /**
@@ -750,14 +751,12 @@ public class ContentProviderUtils {
     }
 
     /**
-     * Creates a location object from a cursor.
+     * Creates a {@link TrackPoint} object from a cursor.
      *
      * @param cursor the cursor pointing to the location
      */
     public TrackPoint createTrackPoint(Cursor cursor) {
-        TrackPoint location = new TrackPoint();
-        fillTrackPoint(cursor, new CachedTrackPointsIndexes(cursor), location);
-        return location;
+        return fillTrackPoint(cursor, new CachedTrackPointsIndexes(cursor));
     }
 
     /**
@@ -868,16 +867,15 @@ public class ContentProviderUtils {
      * Creates a new read-only iterator over a given track's points.
      * It provides a lightweight way of iterating over long tracks without failing due to the underlying cursor limitations.
      * Since it's a read-only iterator, {@link Iterator#remove()} always throws {@link UnsupportedOperationException}.
-     * Each call to {@link TrackPointIterator#next()} may advance to the next DB record, and if so, the iterator calls {@link TrackPointFactory#create()} and populates it with information retrieved from the record.
+     * Each call to {@link TrackPointIterator#next()} may advance to the next DB record.
      * When done with iteration, {@link TrackPointIterator#close()} must be called.
      *
      * @param trackId           the track id
      * @param startTrackPointId the starting track point id. -1L to ignore
      * @param descending        true to sort the result in descending order (latest location first)
-     * @param trackPointFactory   the location factory
      */
-    public TrackPointIterator getTrackPointLocationIterator(final long trackId, final long startTrackPointId, final boolean descending, final TrackPointFactory trackPointFactory) {
-        return new TrackPointIterator(this, trackId, startTrackPointId, descending, trackPointFactory);
+    public TrackPointIterator getTrackPointLocationIterator(final long trackId, final long startTrackPointId, final boolean descending) {
+        return new TrackPointIterator(this, trackId, startTrackPointId, descending);
     }
 
     private TrackPoint findTrackPointBy(String selection, String[] selectionArgs) {

src/main/java/de/dennisguse/opentracks/content/provider/TrackPointFactory.java

@@ -1,22 +0,0 @@
-package de.dennisguse.opentracks.content.provider;
-
-import android.location.Location;
-import android.location.LocationManager;
-
-import de.dennisguse.opentracks.content.data.TrackPoint;
-
-/**
- * Creates a new {@link TrackPoint}.
- * An implementation can create new instances or reuse existing instances for optimization.
- */
-public class TrackPointFactory {
-
-    /**
-     * The default {@link TrackPointFactory} which creates a location each time.
-     */
-    public static final TrackPointFactory DEFAULT_LOCATION_FACTORY = new TrackPointFactory();
-
-    public TrackPoint create() {
-        return new TrackPoint(new Location(LocationManager.GPS_PROVIDER));
-    }
-}

src/main/java/de/dennisguse/opentracks/content/provider/TrackPointIterator.java

@@ -18,21 +18,15 @@ public class TrackPointIterator implements Iterator<TrackPoint>, AutoCloseable {
     private final ContentProviderUtils contentProviderUtils;
     private final long trackId;
     private final boolean descending;
-    private final TrackPointFactory trackPointFactory; //TODO Remove; seems to be an old performance optimization.
     private final CachedTrackPointsIndexes indexes;
     private long lastTrackPointId = -1L;
     private Cursor cursor;
 
 
-    public TrackPointIterator(ContentProviderUtils contentProviderUtils, long trackId, long startTrackPointId, boolean descending, TrackPointFactory trackPointFactory) {
-        if (trackPointFactory == null) {
-            throw new IllegalArgumentException("trackPointFactory is null");
-        }
-
+    public TrackPointIterator(ContentProviderUtils contentProviderUtils, long trackId, long startTrackPointId, boolean descending) {
         this.contentProviderUtils = contentProviderUtils;
         this.trackId = trackId;
         this.descending = descending;
-        this.trackPointFactory = trackPointFactory;
 
         cursor = getCursor(startTrackPointId);
         indexes = cursor != null ? new CachedTrackPointsIndexes(cursor)
@@ -91,9 +85,7 @@ public class TrackPointIterator implements Iterator<TrackPoint>, AutoCloseable {
             }
         }
         lastTrackPointId = cursor.getLong(indexes.idIndex);
-        TrackPoint trackPoint = trackPointFactory.create();
-        ContentProviderUtils.fillTrackPoint(cursor, indexes, trackPoint);
-        return trackPoint;
+        return ContentProviderUtils.fillTrackPoint(cursor, indexes);
     }
 
     @Override

src/main/java/de/dennisguse/opentracks/io/file/exporter/FileTrackExporter.java

@@ -28,7 +28,6 @@ import de.dennisguse.opentracks.content.data.Track;
 import de.dennisguse.opentracks.content.data.TrackPoint;
 import de.dennisguse.opentracks.content.data.Waypoint;
 import de.dennisguse.opentracks.content.provider.ContentProviderUtils;
-import de.dennisguse.opentracks.content.provider.TrackPointFactory;
 import de.dennisguse.opentracks.content.provider.TrackPointIterator;
 import de.dennisguse.opentracks.util.LocationUtils;
 
@@ -128,7 +127,7 @@ public class FileTrackExporter implements TrackExporter {
         int locationNumber = 0;
         TrackPoint lastTrackPoint = null;
 
-        try (TrackPointIterator trackPointIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY)) {
+        try (TrackPointIterator trackPointIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false)) {
 
             while (trackPointIterator.hasNext()) {
                 if (Thread.interrupted()) {

src/main/java/de/dennisguse/opentracks/io/file/importer/AbstractFileTrackImporter.java

@@ -40,7 +40,6 @@ import de.dennisguse.opentracks.content.data.Track;
 import de.dennisguse.opentracks.content.data.TrackPoint;
 import de.dennisguse.opentracks.content.data.Waypoint;
 import de.dennisguse.opentracks.content.provider.ContentProviderUtils;
-import de.dennisguse.opentracks.content.provider.TrackPointFactory;
 import de.dennisguse.opentracks.content.provider.TrackPointIterator;
 import de.dennisguse.opentracks.stats.TripStatisticsUpdater;
 import de.dennisguse.opentracks.util.FileUtils;
@@ -161,7 +160,7 @@ abstract class AbstractFileTrackImporter extends DefaultHandler implements Track
         @Deprecated // TODO Should not be necessary anymore?
                 TripStatisticsUpdater markerTripStatisticsUpdater = new TripStatisticsUpdater(track.getTripStatistics().getStartTime());
 
-        try (TrackPointIterator trackPointIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY)) {
+        try (TrackPointIterator trackPointIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false)) {
 
             while (true) {
                 if (waypoint == null) {

src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java

@@ -46,7 +46,6 @@ import de.dennisguse.opentracks.content.data.TrackPoint;
 import de.dennisguse.opentracks.content.data.Waypoint;
 import de.dennisguse.opentracks.content.provider.ContentProviderUtils;
 import de.dennisguse.opentracks.content.provider.CustomContentProvider;
-import de.dennisguse.opentracks.content.provider.TrackPointFactory;
 import de.dennisguse.opentracks.content.provider.TrackPointIterator;
 import de.dennisguse.opentracks.content.sensor.SensorDataSet;
 import de.dennisguse.opentracks.services.sensors.BluetoothRemoteSensorManager;
@@ -378,7 +377,7 @@ public class TrackRecordingService extends Service {
         TripStatistics tripStatistics = track.getTripStatistics();
         trackTripStatisticsUpdater = new TripStatisticsUpdater(tripStatistics.getStartTime());
 
-        try (TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false, TrackPointFactory.DEFAULT_LOCATION_FACTORY)) {
+        try (TrackPointIterator locationIterator = contentProviderUtils.getTrackPointLocationIterator(track.getId(), -1L, false)) {
             trackTripStatisticsUpdater.addTrackPoint(locationIterator, recordingDistanceInterval);
         } catch (RuntimeException e) {
             Log.e(TAG, "RuntimeException", e);