Skip to content

Conversation

@iseeberg79
Copy link
Contributor

O(n²) -> O(n)

@evcc-bot evcc-bot added enhancement New feature or request infrastructure Basic functionality labels Jan 2, 2026
@iseeberg79 iseeberg79 marked this pull request as ready for review January 2, 2026 23:38
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • Changing continue to return alters the control flow from skipping the rest of the loop iteration to exiting clampRatesSeq entirely; if the intent is just to stop iterating further, consider returning from the iterator callback or breaking out of the loop more locally so the function’s behavior remains clearer to readers.
  • Given the claim of reducing complexity from O(n²) to O(n), consider adding a brief code comment near the early-exit condition explaining why no further work is needed once !r.Start.Before(end) is hit, to make the short-circuiting behavior more obvious.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Changing `continue` to `return` alters the control flow from skipping the rest of the loop iteration to exiting `clampRatesSeq` entirely; if the intent is just to stop iterating further, consider returning from the iterator callback or breaking out of the loop more locally so the function’s behavior remains clearer to readers.
- Given the claim of reducing complexity from O(n²) to O(n), consider adding a brief code comment near the early-exit condition explaining why no further work is needed once `!r.Start.Before(end)` is hit, to make the short-circuiting behavior more obvious.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@andig
Copy link
Member

andig commented Jan 3, 2026

go test -bench=. -count 10 > old

# checkout
go test -bench=. -count 10 > new

benchstat old new

goos: darwin
goarch: arm64
pkg: github.com/evcc-io/evcc/core/planner
cpu: Apple M4 Pro
                        │     old      │                 new                 │
                        │    sec/op    │   sec/op     vs base                │
FindContinuousWindow-12   21.126µ ± 1%   6.449µ ± 1%  -69.47% (p=0.000 n=10)
OptimalPlan-12             68.80n ± 1%   68.64n ± 1%        ~ (p=0.469 n=10)
geomean                    1.206µ        665.4n       -44.81%

Oh yeah...

@andig andig merged commit 8734635 into evcc-io:master Jan 3, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request infrastructure Basic functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants