Move .merge() from SegmentStatisticUpdater to Statistics.

This is a functional change as maxSpeed is now determined by Segment (rather than the joined Segment).

src/androidTest/java/de/dennisguse/opentracks/data/models/StatisticsTest.java

@@ -0,0 +1,65 @@
+package de.dennisguse.opentracks.data.models;
+
+import static org.junit.Assert.assertEquals;
+
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.time.Duration;
+import java.time.Instant;
+
+@RunWith(AndroidJUnit4.class)
+public class StatisticsTest {
+
+    @Test
+    public void testMerge() {
+        // given
+        Statistics subject = new Statistics(
+                Instant.ofEpochMilli(1000),
+                Instant.ofEpochMilli(2500),
+                Duration.ofMillis(1500),
+                Duration.ofMillis(700),
+                Distance.of(750.0),
+                Speed.of(60.0),
+                new AltitudeExtremities(1200, 1250),
+                new AltitudeGainLoss(50, 0),
+                HeartRate.of(100),
+                null
+        );
+
+        Statistics append = new Statistics(
+                        Instant.ofEpochMilli(3000),
+                        Instant.ofEpochMilli(4000),
+                        Duration.ofMillis(1000),
+                        Duration.ofMillis(600),
+                        Distance.of(350.0),
+                        Speed.of(30.0),
+                        new AltitudeExtremities(2800.0, 3575.0),
+                        new AltitudeGainLoss(850, 0),
+                        HeartRate.of(200),
+                        null
+        );
+
+        // when
+        Statistics result = subject.merge(append);
+
+        // then
+        assertEquals(
+                new Statistics(
+                        Instant.ofEpochMilli(1000),
+                        Instant.ofEpochMilli(4000),
+                        Duration.ofMillis(2500),
+                        Duration.ofMillis(1300),
+                        Distance.of(1100),
+                        Speed.of(60),
+                        new AltitudeExtremities(1200, 3575),
+                        new AltitudeGainLoss(900, 0),
+                        HeartRate.of(150),
+                        null
+                )
+                , result
+        );
+    }
+}

src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java

@@ -216,7 +216,7 @@ public class ExportImportTest {
         assertEquals(222049.34375, trackStatistics.totalDistance().toM(), 0.01); //TODO Too low
 
         // Speed
-        assertEquals(8540.359, trackStatistics.maxSpeed().toMPS(), 0.01);
+        assertEquals(22203.7421875, trackStatistics.maxSpeed().toMPS(), 0.01);
         assertEquals(3965.166, trackStatistics.getAverageSpeed().toMPS(), 0.01);
         assertEquals(8540.359, trackStatistics.getAverageMovingSpeed().toMPS(), 0.01);
 
@@ -340,7 +340,7 @@ public class ExportImportTest {
         assertEquals(222049.421, importedTrackStatistics.totalDistance().toM(), 0.01);
 
         // Speed
-        assertEquals(8540.362, importedTrackStatistics.maxSpeed().toMPS(), 0.01);
+        assertEquals(22203.7421875, importedTrackStatistics.maxSpeed().toMPS(), 0.01);
         assertEquals(3965.168, importedTrackStatistics.getAverageSpeed().toMPS(), 0.01);
         assertEquals(8540.362, importedTrackStatistics.getAverageMovingSpeed().toMPS(), 0.01);
 

src/androidTest/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdaterTest.java

@@ -1,127 +0,0 @@
-/*
- * Copyright 2010 Google Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License"); you may not
- * use this file except in compliance with the License. You may obtain a copy of
- * the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations under
- * the License.
- */
-package de.dennisguse.opentracks.stats;
-
-import static org.junit.Assert.assertEquals;
-
-import androidx.test.ext.junit.runners.AndroidJUnit4;
-
-import org.junit.Test;
-import org.junit.runner.RunWith;
-
-import java.time.Duration;
-import java.time.Instant;
-
-import de.dennisguse.opentracks.data.models.AltitudeExtremities;
-import de.dennisguse.opentracks.data.models.AltitudeGainLoss;
-import de.dennisguse.opentracks.data.models.Distance;
-import de.dennisguse.opentracks.data.models.HeartRate;
-import de.dennisguse.opentracks.data.models.Speed;
-import de.dennisguse.opentracks.data.models.Statistics;
-
-/**
- * Tests for {@link SegmentStatisticUpdater}.
- * This only tests non-trivial pieces of that class.
- *
- * @author Rodrigo Damazio
- */
-@RunWith(AndroidJUnit4.class)
-public class SegmentStatisticUpdaterTest {
-
-    @Deprecated //TODO SegmentStatisticsUpdater should always have data, right?
-    @Test
-    public void testMerge_no_data() {
-        // given
-        SegmentStatisticUpdater subject = new SegmentStatisticUpdater();
-        SegmentStatisticUpdater append = new SegmentStatisticUpdater();
-
-        // when
-        subject.merge(append);
-        Statistics result = subject.getStatistics();
-
-        // then
-        assertEquals(
-                new Statistics(
-                        null,
-                        null,
-                        Duration.ZERO,
-                        Duration.ZERO,
-                        Distance.ZERO,
-                        Speed.ZERO,
-                        null,
-                        null,
-                        null,
-                        null
-                ),
-                result
-        );
-    }
-
-    @Test
-    public void testMerge() {
-        // given
-        SegmentStatisticUpdater subject = new SegmentStatisticUpdater(
-                new Statistics(
-                        Instant.ofEpochMilli(1000),
-                        Instant.ofEpochMilli(2500),
-                        Duration.ofMillis(1500),
-                        Duration.ofMillis(700),
-                        Distance.of(750.0),
-                        Speed.of(60.0),
-                        new AltitudeExtremities(1200, 1250),
-                        new AltitudeGainLoss(50, 0),
-                        HeartRate.of(100),
-                        null
-                )
-        );
-
-        SegmentStatisticUpdater append = new SegmentStatisticUpdater(
-                new Statistics(
-                        Instant.ofEpochMilli(3000),
-                        Instant.ofEpochMilli(4000),
-                        Duration.ofMillis(1000),
-                        Duration.ofMillis(600),
-                        Distance.of(350.0),
-                        Speed.of(30.0),
-                        new AltitudeExtremities(2800.0, 3575.0),
-                        new AltitudeGainLoss(850, 0),
-                        HeartRate.of(200),
-                        null
-                )
-        );
-
-        // when
-        subject.merge(append);
-        Statistics result = subject.getStatistics();
-
-        // then
-        assertEquals(
-                new Statistics(
-                        Instant.ofEpochMilli(1000),
-                        Instant.ofEpochMilli(4000),
-                        Duration.ofMillis(2500),
-                        Duration.ofMillis(1300),
-                        Distance.of(1100),
-                        Speed.of(846.1538461538461), //TODO Check why this is not 60
-                        new AltitudeExtremities(1200, 3575),
-                        new AltitudeGainLoss(900, 0),
-                        HeartRate.of(150),
-                        null
-                )
-                , result
-        );
-    }
-}

src/main/java/de/dennisguse/opentracks/data/models/Statistics.java

@@ -1,9 +1,11 @@
 package de.dennisguse.opentracks.data.models;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Objects;
 
 //TODO Add @NonNull to attributes
 public record Statistics(
@@ -54,4 +56,73 @@ public record Statistics(
         if (altitudeExtremities == null) return 0;
         return altitudeExtremities.max_m();
     }
+
+    /**
+     * Combines these statistics with those from another object.
+     * This assumes that the time periods covered by each do not intersect.
+     */
+    //TODO Should be refactored to append only
+    @NonNull
+    public Statistics merge(@NonNull Statistics other) {
+        HeartRate newAvgHeartRate = avgHeartRate;
+        if (avgHeartRate == null) {
+            newAvgHeartRate = other.avgHeartRate;
+        } else {
+            if (other.avgHeartRate != null) {
+                // Using total time as weights for the averaging.
+                // Important to do this before total time is updated
+                newAvgHeartRate = HeartRate.of(
+                        (totalDuration.getSeconds() * avgHeartRate.getBPM() + other.totalDuration.getSeconds() * other.avgHeartRate.getBPM())
+                                / (totalDuration.getSeconds() + other.totalDuration.getSeconds())
+                );
+            }
+        }
+
+        Power newAvgPower = avgPower;
+        if (avgPower == null) {
+            newAvgPower = other.avgPower;
+        } else {
+            if (other.avgPower != null) {
+                // Using total time as weights for the averaging.
+                // Important to do this before total time is updated
+                newAvgPower = Power.of(
+                        (totalDuration.getSeconds() * avgPower.getW() + other.totalDuration.getSeconds() * other.avgPower.getW())
+                                / (totalDuration.getSeconds() + other.totalDuration.getSeconds())
+                );
+            }
+        }
+
+        AltitudeExtremities newAltitudeExtremities;
+        if (altitudeExtremities != null && other.altitudeExtremities != null) {
+            newAltitudeExtremities = new AltitudeExtremities(
+                    Math.min(altitudeExtremities.min_m(), other.altitudeExtremities.min_m()),
+                    Math.max(altitudeExtremities.max_m(), other.altitudeExtremities.max_m())
+            );
+        } else {
+            newAltitudeExtremities = altitudeExtremities != null ? altitudeExtremities : other.altitudeExtremities;
+        }
+
+        AltitudeGainLoss newAltitudeGainLoss;
+        if (altitudeGainLoss != null && other.altitudeGainLoss != null) {
+            newAltitudeGainLoss = new AltitudeGainLoss(
+                    altitudeGainLoss.gain_m() + other.altitudeGainLoss.gain_m(),
+                    altitudeGainLoss.loss_m() + other.altitudeGainLoss.loss_m()
+            );
+        } else {
+            newAltitudeGainLoss = altitudeGainLoss != null ? altitudeGainLoss : other.altitudeGainLoss;
+        }
+
+        return new Statistics(
+                startTime == null ? other.startTime : startTime.isBefore(other.startTime) ? startTime : other.startTime,
+                stopTime == null ? other.stopTime : stopTime.isAfter(other.stopTime) ? stopTime : other.stopTime,
+                totalDuration.plus(other.totalDuration),
+                movingDuration.plus(other.movingDuration),
+                totalDistance.plus(other.totalDistance),
+                Speed.max(maxSpeed, other.maxSpeed),
+                newAltitudeExtremities,
+                newAltitudeGainLoss,
+                newAvgHeartRate,
+                newAvgPower
+                );
+    }
 }

src/main/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdater.java

@@ -71,6 +71,10 @@ public class SegmentStatisticUpdater {
         reset();
     }
 
+    public SegmentStatisticUpdater(Instant startTime) {
+        reset(startTime);
+    }
+
     /**
      * Copy constructor.
      *
@@ -98,81 +102,8 @@ public class SegmentStatisticUpdater {
         avgPower = statistics.avgPower();
     }
 
-    public Statistics aggregate(Statistics statistics) {
+    public Statistics merge(Statistics statistics) {
-        SegmentStatisticUpdater intermediate = new SegmentStatisticUpdater(statistics);
+        return statistics.merge(getStatistics());
-        intermediate.merge(this);
-        return intermediate.getStatistics();
-    }
-
-    /**
-     * Combines these statistics with those from another object.
-     * This assumes that the time periods covered by each do not intersect.
-     */
-    //TODO Should be refactored to append only [mainly due to isIdle]
-    public void merge(SegmentStatisticUpdater other) {
-        if (startTime == null) {
-            startTime = other.startTime;
-        } else {
-            startTime = startTime.isBefore(other.startTime) ? startTime : other.startTime;
-        }
-        if (stopTime == null) {
-            stopTime = other.stopTime;
-        } else {
-            stopTime = stopTime.isAfter(other.stopTime) ? stopTime : other.stopTime;
-        }
-
-        if (avgHeartRate == null) {
-            avgHeartRate = other.avgHeartRate;
-        } else {
-            if (other.avgHeartRate != null) {
-                // Using total time as weights for the averaging.
-                // Important to do this before total time is updated
-                avgHeartRate = HeartRate.of(
-                        (totalDuration.getSeconds() * avgHeartRate.getBPM() + other.totalDuration.getSeconds() * other.avgHeartRate.getBPM())
-                                / (totalDuration.getSeconds() + other.totalDuration.getSeconds())
-                );
-            }
-        }
-
-        if (avgPower == null) {
-            avgPower = other.avgPower;
-        } else {
-            if (other.avgPower != null) {
-                // Using total time as weights for the averaging.
-                // Important to do this before total time is updated
-                avgPower = Power.of(
-                        (totalDuration.getSeconds() * avgPower.getW() + other.totalDuration.getSeconds() * other.avgPower.getW())
-                                / (totalDuration.getSeconds() + other.totalDuration.getSeconds())
-                );
-            }
-        }
-
-        totalDistance = totalDistance.plus(other.totalDistance);
-        totalDuration = totalDuration.plus(other.totalDuration);
-        movingDuration = movingDuration.plus(other.movingDuration);
-        maxSpeed = Speed.max(maxSpeed, other.maxSpeed);
-        if (other.altitudeExtremities.hasData()) {
-            altitudeExtremities.update(other.altitudeExtremities.getMin());
-            altitudeExtremities.update(other.altitudeExtremities.getMax());
-        }
-        if (totalAltitudeGain_m == null) {
-            if (other.totalAltitudeGain_m != null) {
-                totalAltitudeGain_m = other.totalAltitudeGain_m;
-            }
-        } else {
-            if (other.totalAltitudeGain_m != null) {
-                totalAltitudeGain_m += other.totalAltitudeGain_m;
-            }
-        }
-        if (totalAltitudeLoss_m == null) {
-            if (other.totalAltitudeLoss_m != null) {
-                totalAltitudeLoss_m = other.totalAltitudeLoss_m;
-            }
-        } else {
-            if (other.totalAltitudeLoss_m != null) {
-                totalAltitudeLoss_m += other.totalAltitudeLoss_m;
-            }
-        }
     }
 
     public boolean isInitialized() {

src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java

@@ -94,7 +94,7 @@ public class TrackStatisticsUpdater {
      * Compute TrackStatistics.
      */
     public Statistics getTrackStatistics() {
-        return currentSegment.aggregate(statisticsWithoutCurrentSegment);
+        return currentSegment.merge(statisticsWithoutCurrentSegment);
     }
 
     public boolean isIdle() {
@@ -200,7 +200,7 @@ public class TrackStatisticsUpdater {
 
     private void reset(TrackPoint trackPoint) {
         if (currentSegment.isInitialized()) {
-            statisticsWithoutCurrentSegment = currentSegment.aggregate(statisticsWithoutCurrentSegment);
+            statisticsWithoutCurrentSegment = currentSegment.merge(statisticsWithoutCurrentSegment);
         }
         currentSegment.reset(trackPoint.getTime());