From 73d6ca6ca68e71de9ca450ddafa30c7cb8869d41 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Fri, 21 Nov 2025 16:19:32 +0100 Subject: [PATCH] Cleanup: SegmentStatisticUpdater is always initialized. --- .../stats/TrackStatisticsUpdaterTest.java | 4 +- .../opentracks/data/models/Statistics.java | 23 ++++++++- .../opentracks/data/models/Track.java | 3 +- .../opentracks/services/RecordingData.java | 2 +- .../stats/SegmentStatisticUpdater.java | 49 +++++-------------- .../stats/TrackStatisticsUpdater.java | 24 ++++----- 6 files changed, 48 insertions(+), 57 deletions(-) diff --git a/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java b/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java index e74b68255..d9a136a8c 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java @@ -166,14 +166,14 @@ public class TrackStatisticsUpdaterTest { assertEquals(Speed.of(2), subject.getTrackStatistics().maxSpeed()); // when - subject.addTrackPoints(List.of( + List.of( new TrackPoint(TrackPoint.Type.SEGMENT_START_MANUAL, Instant.ofEpochSecond(5)), createTrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(6)) .setSpeed(Speed.of(1f)), createTrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(7)) .setSpeed(Speed.of(1f)), new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL, Instant.ofEpochSecond(8)) - )); + ).forEach(subject::addTrackPoint); // then assertEquals(Speed.of(2f), subject.getTrackStatistics().maxSpeed()); diff --git a/src/main/java/de/dennisguse/opentracks/data/models/Statistics.java b/src/main/java/de/dennisguse/opentracks/data/models/Statistics.java index ce4d00863..cc8579d86 100644 --- a/src/main/java/de/dennisguse/opentracks/data/models/Statistics.java +++ b/src/main/java/de/dennisguse/opentracks/data/models/Statistics.java @@ -7,15 +7,20 @@ import java.time.Duration; import java.time.Instant; import java.util.Objects; -//TODO Add @NonNull to attributes public record Statistics( + @NonNull Instant startTime, + @NonNull Instant stopTime, + @NonNull Duration totalDuration, + @NonNull Duration movingDuration, // Based on when we believe the user is traveling + @NonNull Distance totalDistance, + @NonNull Speed maxSpeed, @Nullable @@ -31,6 +36,22 @@ public record Statistics( Power avgPower ) { + //TODO: Should not be necessary; refactor and remove. + @Deprecated + public static final Statistics DEFAULT = + new Statistics( + null, + null, + Duration.ZERO, + Duration.ZERO, + Distance.ZERO, + Speed.ZERO, + null, + null, + null, + null + ); + public Duration getStoppedTime() { return totalDuration.minus(movingDuration); } diff --git a/src/main/java/de/dennisguse/opentracks/data/models/Track.java b/src/main/java/de/dennisguse/opentracks/data/models/Track.java index 5fb13f762..8044d9cba 100644 --- a/src/main/java/de/dennisguse/opentracks/data/models/Track.java +++ b/src/main/java/de/dennisguse/opentracks/data/models/Track.java @@ -24,6 +24,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; +import java.time.Duration; import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.UUID; @@ -67,7 +68,7 @@ public class Track { @Deprecated //TODO Remove public Track(@NonNull ZoneOffset zoneOffset) { - this(zoneOffset, new SegmentStatisticUpdater().getStatistics()); + this(zoneOffset, Statistics.DEFAULT); } public Track(@NonNull ZoneOffset zoneOffset, @NonNull Statistics trackStatistics) { diff --git a/src/main/java/de/dennisguse/opentracks/services/RecordingData.java b/src/main/java/de/dennisguse/opentracks/services/RecordingData.java index 2d4a57512..ff6895f70 100644 --- a/src/main/java/de/dennisguse/opentracks/services/RecordingData.java +++ b/src/main/java/de/dennisguse/opentracks/services/RecordingData.java @@ -23,7 +23,7 @@ public record RecordingData(Track track, TrackPoint latestTrackPoint, SensorData @NonNull public Statistics getStatisticsTrack() { if (track == null) { - return new SegmentStatisticUpdater().getStatistics(); + return Statistics.DEFAULT; //TODO Refactor code that this is not necessary. } return track.getStatistics(); diff --git a/src/main/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdater.java b/src/main/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdater.java index e6851320b..b0a62a9fa 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdater.java +++ b/src/main/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdater.java @@ -46,9 +46,11 @@ public class SegmentStatisticUpdater { private final ExtremityMonitor altitudeExtremities = new ExtremityMonitor(); // The track start time. - private Instant startTime; //TODO Should never be null! + @NonNull + private final Instant startTime; // The track stop time. - private Instant stopTime; //TODO Should never be null! + @NonNull + private Instant stopTime; private Distance totalDistance; /** @@ -67,12 +69,14 @@ public class SegmentStatisticUpdater { private HeartRate avgHeartRate = null; private Power avgPower = null; - public SegmentStatisticUpdater() { - reset(); - } - - public SegmentStatisticUpdater(Instant startTime) { - reset(startTime); + public SegmentStatisticUpdater(@NonNull Instant startTime) { + this.startTime = this.stopTime = startTime; + totalDuration = Duration.ZERO; + movingDuration = Duration.ZERO; + totalDistance = Distance.ZERO; + maxSpeed = Speed.ZERO; + totalAltitudeGain_m = null; + totalAltitudeLoss_m = null; } /** @@ -106,27 +110,6 @@ public class SegmentStatisticUpdater { return statistics.merge(getStatistics()); } - public boolean isInitialized() { - return startTime != null; - } - - public void reset() { - startTime = null; - stopTime = null; - - totalDuration = Duration.ZERO; - movingDuration = Duration.ZERO; - totalDistance = Distance.ZERO; - maxSpeed = Speed.ZERO; - totalAltitudeGain_m = null; - totalAltitudeLoss_m = null; - } - - public void reset(Instant startTime) { - reset(); - setStartTime(startTime); - } - public Statistics getStatistics() { // Times may not be live (i.e., updated automatically). return new Statistics( @@ -146,14 +129,6 @@ public class SegmentStatisticUpdater { ); } - /** - * Should only be called on start. - */ - public void setStartTime(Instant startTime) { - this.startTime = startTime; - setStopTime(startTime); - } - public void setStopTime(Instant stopTime) { if (stopTime.isBefore(startTime)) { // Time must be monotonically increasing, but we might have events at the same point in time (BLE and GPS) diff --git a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java index a0cca7182..37f7c3eef 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java +++ b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java @@ -32,8 +32,7 @@ import de.dennisguse.opentracks.settings.PreferencesUtils; /** * Updater for {@link SegmentStatisticUpdater}. * For updating track {@link SegmentStatisticUpdater} as new {@link TrackPoint}s are added. - * NOTE: Some of the locations represent pause/resume separator. - * NOTE: Has still support for segments (at the moment unused). + * NOTE: {@link TrackPoint} represent pause/resume separator. * * @author Sandor Dornbush * @author Rodrigo Damazio @@ -51,7 +50,7 @@ public class TrackStatisticsUpdater { private Duration totalPowerDuration = Duration.ZERO; // The current segment's statistics - private final SegmentStatisticUpdater currentSegment; + private SegmentStatisticUpdater currentSegment; // Current segment's last trackPoint private TrackPoint lastTrackPoint; @@ -59,7 +58,7 @@ public class TrackStatisticsUpdater { @Deprecated public TrackStatisticsUpdater() { - this(new SegmentStatisticUpdater().getStatistics()); + this(Statistics.DEFAULT); } public TrackStatisticsUpdater(@NonNull TrackPoint trackPoint) { @@ -71,12 +70,12 @@ public class TrackStatisticsUpdater { this(); assert !trackPoints.isEmpty(); //TODO Enforce that this is always true (e.g., import) - addTrackPoints(trackPoints); + trackPoints.forEach(this::addTrackPoint); } public TrackStatisticsUpdater(@NonNull Statistics statistics) { this.statisticsWithoutCurrentSegment = statistics; - this.currentSegment = new SegmentStatisticUpdater(); + this.currentSegment = null; resetAverageHeartRate(); } @@ -105,17 +104,12 @@ public class TrackStatisticsUpdater { return currentSegment.getStatistics(); } - public void addTrackPoints(List trackPoints) { - trackPoints.forEach(this::addTrackPoint); - } - public void addTrackPoint(TrackPoint trackPoint) { if (trackPoint.isSegmentManualStart()) { reset(trackPoint); } - - if (!currentSegment.isInitialized()) { - currentSegment.setStartTime(trackPoint.getTime()); + if (currentSegment == null) { + currentSegment = new SegmentStatisticUpdater(trackPoint.getTime()); } // Always update time @@ -199,10 +193,10 @@ public class TrackStatisticsUpdater { } private void reset(TrackPoint trackPoint) { - if (currentSegment.isInitialized()) { + if (currentSegment != null) { statisticsWithoutCurrentSegment = currentSegment.merge(statisticsWithoutCurrentSegment); } - currentSegment.reset(trackPoint.getTime()); + currentSegment = new SegmentStatisticUpdater(trackPoint.getTime()); lastTrackPoint = null; resetAverageHeartRate();