Skip to content

REFACTOR_PLAN.md

Latest commit

 

History

History
99 lines (81 loc) · 8.85 KB

REFACTOR_PLAN.md

File metadata and controls

99 lines (81 loc) · 8.85 KB
Raw

Refactor / Performance / Cleanup Plan

Fresh-eyes audit (2026-06-06) of MaximumTrainer — app originally written ~2014, migrated to Qt6 / C++17, so some parts are dated. Sequenced as small, independent, verifiable PRs. Verify each finding against current code before acting — line numbers drift as the tree changes.

Status legend: ☐ todo · ◐ in progress · ☑ done

Progress snapshot (updated 2026-06-10)

Group Status PR
1 — Dead-code sweep ☑ done #230
2 — Hot-path guards ☑ done (2 of 4 items; 2 evaluated and intentionally skipped) #227
3 — De-duplication via templates ◐ partial (singletons done; editor step-type helper shipped #239) #236, #239
4 — Latent naming hazard ☑ done #235
5 — Quick correctness fixes ☑ done (1 of 3; 2 dropped after inspection) #234
6 — Large mechanical modernization ☐ todo —
7 — Dead maximumtrainer.com backend ☑ done #308

Related work already merged: QWT 6.3 bump, AppImage dark-mode + OpenSSL fixes, QwtSystemClock → QElapsedTimer (#228), CLAUDE.md (#229). Adjacent cleanup that has since landed: dead PowerCurve UI + sensor/trainer config consolidation (#241, #242), ANT+ remnant removal (#240), and removal of the SelfLoops/ TrainingPeaks integrations (#244) and the dead web frontend (#245).

Two large features also shipped in this window (tracked outside this plan, now done): Strava auto-upload (#246) and native Studio mode (#253). Their planning docs (STRAVA_INTEGRATION_PLAN.md, STUDIO_MODE_TODO.md) were removed once complete — see git history if you need them.

Group 1 — Dead-code sweep ☑

Pure deletion, zero behavioural risk. Each target verified unreferenced first.

  • ☑ fitactivitycreator.cpp build_FIT_file() — dead method, never called, contained a hardcoded developer path /Users/tourlou2/test2.fit. Removed (+ commented decode_FIT_file).
  • ☑ xmlstreamwritertcx.{h,cpp} — entirely commented out, not in any .pri. Removed both files + the dead #include in dataworkout.cpp.
  • ☑ Course feature removed end to end (~2,200 lines): course.*, coursetablemodel.*, sortfilterproxymodelcourse.*, main_coursepage.*, googlemapwidget.*, plus surgical edits to util / account / settings / environnement / userdao / xmlutil / mainwindow / dialogmainwindowconfig / apptheme / z_stylesheet and the 5 integration-test .pro files. Kept: FIT-SDK course messages and the .workout-format "COURSE DATA" parser. Removed: gpxparser.

Group 2 — Hot-path guards (perf during a workout) ☑ (PR #227)

Built on the mini-graph throttle. Shipped the two real wins; the other two audit items were evaluated against the actual data bounds and intentionally skipped (risk to core data paths for no measurable gain).

  • ☑ WorkoutPlotZoomer::updateTextLabelValue & MinimalistWidget::setValue — setStyleSheet() was called on every BLE packet (full style recompute) even when the colour was unchanged. Both now guard on the current colour, mirroring InfoWidget::currentColor.
  • ☑ Workout::getLstInterval() now returns const QList<Interval>& instead of by value (was copying the whole interval list in several hot/loop paths). Mutating callers copy locally (COW), so safe.
  • ⏭ Skipped — rolling power running-sum (workoutdialog.cpp): the averaging window is capped at 5 (combobox None/1-5 sec), so the "O(n) re-sum" is ≤5 elements. A running sum would add risk to a core averaging path for no gain.
  • ⏭ Skipped — updateCurve ring buffer (workoutplotzoomer.cpp): the sample buffer is capped at 90 at 1-4 Hz, and Qwt's setSamples copies internally regardless. The original #1 audit ranking conflated this with the 40 Hz replot, which #227 already fixed.

Group 3 — De-duplication via templates ◐ (partial)

  • ☑ DataPower/DataHeartRate/DataCadence/DataSpeed — the four byte-identical singleton .cpp files (~590 lines) replaced by one CRTP base DataMetricSingleton<Derived> in datametric.h; each metric is now an ~11-line final subclass. Net −598 lines. Public API (DataPower::instance().append() etc.) unchanged; verified app builds + workout plots render live data. (The CurveData* wrappers were left as-is — they differ more than the audit implied and aren't pure boilerplate.) (PR #236)
  • ◐ PowerEditor/HrEditor/CadenceEditor — the audit proposed one parameterised MetricEditor, but a full merge is not small/low-risk: the three diverge (Power adds L/R balance + test-interval; Power/HR have a unit toggle, Cadence doesn't) and each has its own .ui with a distinct Ui:: namespace.
    • ☑ First step shipped: extracted the one byte-identical part — the NONE/FLAT/PROGRESSIVE show-hide block in each on_comboBox_*_currentIndexChanged — into MetricEditorVisibility::applyStepTypeVisibility(...) (metriceditorvisibility.h). ~75 lines of triplicated visibility logic → 1 inline helper + 3 call sites. Verified: builds + workout-editor screenshot renders.
    • ☐ Remaining (optional, larger): dedup the unit-toggle handlers (on_comboBox_FTPorWatts / on_comboBox_LTHRorBpm) and/or a fuller shared base. Lower value, more divergence — defer unless it pays off.

Group 4 — Latent naming hazard ☑

  • ☑ Two classes were both named IntervalsIcuService (static intervals_icu_service.* + QObject intervalsicuservice.*), both compiled in db.pri — an ODR/confusion trap. Renamed the static one to IntervalsIcuApi and its files to intervals_icu_api.{h,cpp}; updated db.pri and the 3 test projects that use it. The QObject IntervalsIcuService (used by the UI) is unchanged. Verified: app builds + the intervals_icu_tests unit suite passes (19/19).

Group 5 — Quick correctness fixes ☑ (1 of 3 shipped; 2 dropped after inspection — PR #234)

  • ☑ util.cpp — Util::zipFileToDisk/unzipFile discarded the QFile::open() return → now checked + logged + bailed. (PR #234)
  • ⏭ Dropped — simplecrypt.cpp qrand(): not actually present/broken on the current Qt6 tree; no change needed.
  • ⏭ Dropped — selfloops_service.cpp local gzipCompress: on inspection it diverges enough from Util::zipFileHelperConvertToGzip that folding it in added risk for no real gain.

Group 6 — Large mechanical modernization (one PR per sweep; low-risk, big diff) ☐

  • ☐ 243 string-based connect(SIGNAL/SLOT) → function-pointer syntax (compile-time checked). Worst: workoutdialog.cpp (96), mainwindow.cpp (66).
  • ☐ 45 foreach/Q_FOREACH (deprecated, removed Qt 6.7; hidden container copy) → range-for with const auto&.
  • ☐ 443 qDebug() → the existing LOG_* framework, prioritising hot-path spew (sendLoads, update1sec, raw server dump at slotGetSensorListFinished:~3012).
  • ☐ Header hygiene: account.h is included in 18 headers but most need only Account* → forward-declare; move workoutdialog.h include out of dialogconfig.h.
  • ☐ parent = 0 → nullptr (~60 headers); stringly-typed dispatch ("power"/"hr"/"cad", "workout"/"course") → enums; typedef enum → enum class.
  • ☐ God-file decomposition: workoutdialog.cpp (4,298 lines; 590-line constructor → extract createPairingOverlay/BatteryOverlay/AchievementOverlay/setupClockThread/setupSoundTimers), mainwindow.cpp (2,629).

Group 7 — Dead maximumtrainer.com backend removal ☑ (#308)

The original ~2014 app talked to a PHP REST backend at maximumtrainer.com (api/*_rest/...). That backend no longer exists, so every call against it was dead (silently failing, in the achievement case retry-looping forever).

  • ☑ Removed the dead DAOs: SensorDAO (unused), UserDAO (impl already commented out; only a stale doc reference remained), AchievementDAO.
  • ☑ ManagerAchievement: stripped the remote list-fetch / put / retry slots. The class + the in-workout achievement popup are kept but dormant (the list is never populated, so the size guards short-circuit). Left in place for a future local-only re-implementation.
  • ☑ Environnement: dropped the now-unused getURLEnvironnement() / getURLEnvironnementWS() and the current_env / dev / prod (= maximumtrainer.com) / indexPage constants.
  • ☑ util.cpp: removed the dead response parsers tied to those DAOs — parseJsonObjectAccount, parseJsonSensorList, parseJsonAchievementList, parseJsonAchievementListForUser.
  • Kept (NOT maximumtrainer.com): VersionDAO (GitHub Releases update check), IntervalsIcuDAO (intervals.icu), ExtRequest, and the live Intervals.icu / Strava OAuth code. Left for a later pass: the Firebase plan/workout-creator list fetch in mainwindow.cpp (trainerdb-84bdb.firebaseapp.com), status of that backend not yet confirmed.

Conventions: ship via the fork-PR flow (see CLAUDE.md); keep comments minimal; prefer small PRs that build + verify locally before pushing.