-
Notifications
You must be signed in to change notification settings - Fork 0
Optimize lookup and caching patterns with performance tests #38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5b04cda
d836f0d
fbd10bc
62986af
b1fe12f
5ff8edb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -208,43 +208,31 @@ class WorkoutProvider extends ChangeNotifier { | |
| return false; | ||
| } | ||
|
|
||
| // Check for references in Sessions | ||
| for (var session in _sessions) { | ||
| if (session.exercises.any((e) => e.exerciseId == exerciseId)) { | ||
| debugPrint( | ||
| 'Cannot delete custom exercise: Used in session ${session.id}', | ||
| ); | ||
| return false; | ||
| } | ||
| } | ||
| // Build a set of referenced exercise IDs in a single pass over | ||
| // sessions, routines, targets, and the active workout. | ||
| final referencedIds = <String>{}; | ||
| final referenceReasons = <String, Set<String>>{}; | ||
|
|
||
| // Check for references in Routines | ||
| for (var routine in _routines) { | ||
| if (routine.exerciseIds.contains(exerciseId)) { | ||
| debugPrint( | ||
| 'Cannot delete custom exercise: Used in routine ${routine.name}', | ||
| ); | ||
| return false; | ||
| } | ||
| void addReference(String id, String reason) { | ||
| referencedIds.add(id); | ||
| referenceReasons.putIfAbsent(id, () => {}).add(reason); | ||
| } | ||
|
|
||
| // Check for references in Targets | ||
| for (var target in _targets) { | ||
| if (target.exerciseId == exerciseId) { | ||
| debugPrint( | ||
| 'Cannot delete custom exercise: Used in target ${target.id}', | ||
| ); | ||
| return false; | ||
| } | ||
| for (final s in _sessions) { | ||
| for (final e in s.exercises) addReference(e.exerciseId, '_sessions'); | ||
| } | ||
|
|
||
| // Check for references in active workout | ||
| if (_currentExerciseLogs.any((l) => l.exerciseId == exerciseId)) { | ||
| debugPrint('Cannot delete custom exercise: Used in active workout'); | ||
| return false; | ||
| for (final r in _routines) { | ||
| for (final id in r.exerciseIds) addReference(id, '_routines'); | ||
| } | ||
| for (final t in _targets) addReference(t.exerciseId, '_targets'); | ||
| for (final l in _currentExerciseLogs) addReference(l.exerciseId, '_currentExerciseLogs'); | ||
| if (_activeRoutine != null) { | ||
| for (final id in _activeRoutine!.exerciseIds) addReference(id, '_activeRoutine'); | ||
| } | ||
|
Comment on lines
+221
to
231
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify flutter_lints / analysis_options enables curly_braces_in_flow_control_structures
fd -e yaml analysis_options
fd -e yaml analysis_options --exec cat {}
rg -n 'curly_braces_in_flow_control_structures' || trueRepository: Devasy/Workout-logger Length of output: 1704 🏁 Script executed: # Read the specific lines in workout_provider.dart to verify the violations
sed -n '220,232p' workout-logger/lib/services/workout_provider.dartRepository: Devasy/Workout-logger Length of output: 582 Add curly braces around single-statement flow-control bodies.
🔧 Fix for (final s in _sessions) {
- for (final e in s.exercises) addReference(e.exerciseId, '_sessions');
+ for (final e in s.exercises) {
+ addReference(e.exerciseId, '_sessions');
+ }
}
for (final r in _routines) {
- for (final id in r.exerciseIds) addReference(id, '_routines');
+ for (final id in r.exerciseIds) {
+ addReference(id, '_routines');
+ }
}
- for (final t in _targets) addReference(t.exerciseId, '_targets');
- for (final l in _currentExerciseLogs) addReference(l.exerciseId, '_currentExerciseLogs');
+ for (final t in _targets) {
+ addReference(t.exerciseId, '_targets');
+ }
+ for (final l in _currentExerciseLogs) {
+ addReference(l.exerciseId, '_currentExerciseLogs');
+ }
if (_activeRoutine != null) {
- for (final id in _activeRoutine!.exerciseIds) addReference(id, '_activeRoutine');
+ for (final id in _activeRoutine!.exerciseIds) {
+ addReference(id, '_activeRoutine');
+ }
}🤖 Prompt for AI Agents |
||
| if (_activeRoutine?.exerciseIds.contains(exerciseId) ?? false) { | ||
| debugPrint('Cannot delete custom exercise: Used in active routine'); | ||
|
|
||
| if (referencedIds.contains(exerciseId)) { | ||
| final reasons = referenceReasons[exerciseId]?.join(', ') ?? 'unknown'; | ||
| debugPrint('Cannot delete custom exercise $exerciseId: still referenced in $reasons'); | ||
| return false; | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
@@ -693,19 +681,31 @@ class WorkoutProvider extends ChangeNotifier { | |
| return data; | ||
| } | ||
|
|
||
| /// Get weekly volume by muscle group | ||
| /// Get weekly volume by muscle group. | ||
| /// | ||
| /// Sessions are traversed newest-first and the loop breaks once we reach | ||
| /// a session older than 7 days, so only in-range sessions are visited. | ||
| /// An exercise map is built once at the top for O(1) per-set lookup. | ||
| Map<String, double> getWeeklyVolumeByMuscle() { | ||
| final volumeByMuscle = <String, double>{}; | ||
| final weekAgo = DateTime.now().subtract(const Duration(days: 7)); | ||
|
|
||
| for (var session in _sessions) { | ||
| if (session.date.isBefore(weekAgo)) continue; | ||
| // Pre-build exercise map for O(1) lookup | ||
| final exerciseMap = <String, Exercise>{ | ||
| for (final e in _allExercises) e.id: e, | ||
| }; | ||
|
|
||
| for (var log in session.exercises) { | ||
| final exercise = getExercise(log.exerciseId); | ||
| // Iterate directly since _sessions is maintained newest-first. | ||
| // We use continue rather than break in case of external imports | ||
| // that might temporarily violate the newest-first invariant. | ||
| for (final session in _sessions) { | ||
| if (session.date.isBefore(weekAgo)) continue; | ||
|
|
||
| for (final log in session.exercises) { | ||
| final exercise = exerciseMap[log.exerciseId]; | ||
| if (exercise == null) continue; | ||
|
|
||
| for (var activation in exercise.muscleActivations) { | ||
| for (final activation in exercise.muscleActivations) { | ||
| final muscleVolume = | ||
| log.totalVolume * (activation.activationPercentage / 100); | ||
| volumeByMuscle[activation.muscleGroupId] = | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Reason labels expose private field names in debug output.
'_sessions','_routines','_targets','_currentExerciseLogs','_activeRoutine'are internal field names; the prior per-collection messages used user-facing labels (e.g."sessions","active workout"). The current strings will surface asstill referenced in _sessions, _activeRoutinein logs, which leaks implementation detail and reads worse than the previous variant.🤖 Prompt for AI Agents