Remove static state in SystemClock - #2041
Conversation
|
|
I'll get the CLA thing fixed, already contacted them to enroll our organisation etc. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2041 +/- ##
============================================
- Coverage 68.14% 68.10% -0.04%
+ Complexity 3083 3081 -2
============================================
Files 459 459
Lines 9031 9027 -4
============================================
- Hits 6154 6148 -6
- Misses 2877 2879 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 5 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Simplifies SystemClock by moving its wall-clock reference from static state to each clock instance.
Changes:
- Computes the reference time during construction.
- Uses an immutable instance property in
now().
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| $this->referenceTime = self::calculateReferenceTime( | ||
| microtime(true), | ||
| hrtime(true) | ||
| ); |
In the library code base
SystemClockis used as a singleton (viaClock::getDefault()). So this change will not actually impact much.Practically it simplifies
SystemClockby removing its internal static state, which I consider to be a code smell.We lose monotonicity between different instances, I do not think this is a bad thing, it could actually be a good thing.
For long running PHP processes (for example ReactPHP or other scenarios where a single process handles many requests) we could choose to instantiate a system clock for each request. Having a single reference time at process start means we can drift away from real time because we're not getting resynced with NTP.
FYI: While the code was edited by opencode, this description is handwritten.