Add arithmetic, diff and modify methods to ChronosTime#523
Open
dereuromark wants to merge 4 commits into3.nextfrom
Open
Add arithmetic, diff and modify methods to ChronosTime#523dereuromark wants to merge 4 commits into3.nextfrom
dereuromark wants to merge 4 commits into3.nextfrom
Conversation
Fixes #522. ChronosTime was introduced in 3.x as a standalone class without the arithmetic, difference or modifier helpers that Chronos and ChronosDate compose via traits, leaving a time-only value effectively read-only and breaking the README's `addHours(2)->addMinutes(15)` example. This adds: - addHours/subHours, addMinutes/subMinutes, addSeconds/subSeconds - modify() restricted to time-unit modifiers (hour/minute/second/ microsecond); date-unit modifiers throw InvalidArgumentException - diff() returning a DateInterval (matches DateTimeInterface::diff default of `absolute = false`) - diffInHours/diffInMinutes/diffInSeconds (absolute by default, Chronos convention) - secondsSinceMidnight - closest/farthest/min/max All arithmetic wraps around midnight via the existing `mod()` helper, so adding 2 hours to 23:00 yields 01:00 and subtracting 2 hours from 00:00 yields 22:00. This is documented on each method.
- Cover null/default $other path for min/max and diffIn* - Cover tie-breaking for closest/farthest - Document arithmetic, modify, diff and closest/min/max sections in docs/en/chronos-time.md
There was a problem hiding this comment.
Pull request overview
Adds missing arithmetic, modifier, diff, and comparison helpers to ChronosTime to restore the intended “time-only values” API and align behavior more closely with other Chronos value objects (fixes #522).
Changes:
- Implemented arithmetic helpers (
add*/sub*) using tick-based wraparound semantics. - Added
modify()(time-unit-only) plus diff helpers (diff(),diffIn*(),secondsSinceMidnight()). - Updated README “Time-only Values” example and added PHPUnit coverage for the new API.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/ChronosTime.php |
Adds tick-based arithmetic/diff/modify and min/max/closest/farthest helpers. |
tests/TestCase/ChronosTimeTest.php |
Adds tests covering the new APIs and wraparound behavior. |
README.md |
Updates the “Time-only Values” docs to use the now-available API and correct construction examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Pin UTC timezone for modify() and diff() internal DateTimeImmutable instances so results are independent of the process default timezone and historical offset changes at the 1970-01-01 anchor. - Guard modify() against DateTimeImmutable::modify() returning false on PHP 8.1/8.2 (matches ChronosDate::modify()). - Drop redundant double-clone in min()/max() and return $this directly when it already wins the comparison. - Document midnight wraparound on all add*/sub* helpers, not just addHours().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #522.
ChronosTimewas introduced in 3.x as a brand-new standalone class and — unlikeChronosandChronosDate, which compose behavior via traits (ModifierTrait,DifferenceTrait, …) — it shipped without any arithmetic, diff or modifier helpers. That left a time-only value effectively read-only, and broke the README's "Time-only Values" example:This PR adds the missing API directly to
ChronosTime. Everything operates on the existing internal$tickscounter (microseconds since midnight) and wraps via the existingmod()helper, so noDateTimeImmutableround-trips are needed for arithmetic.Changes
addHours/subHours,addMinutes/subMinutes,addSeconds/subSecondsmodify(string \$modifier)— restricted by regex to time-unit modifiers (hour/minute/second/microsecond, signed integer, optionally chained). Date-unit modifiers (+1 day,next monday, etc.) throwInvalidArgumentExceptionrather than silently returning the wrong result.diff(ChronosTime \$target, bool \$absolute = false)returning aDateInterval— default matchesDateTimeInterface::diff.diffInHours/diffInMinutes/diffInSeconds—\$absolute = trueby default (Chronos convention).secondsSinceMidnightclosest/farthest/min/maxWraparound semantics
All arithmetic is modulo one day, matching the existing setter behavior (
setHours(24) === setHours(0)):23:00 + 2h → 01:0000:00 − 2h → 22:00Each method's docblock calls this out explicitly.
Not included (and why)
diffForHumans,wasWithinLast,isWithinNext— these imply an instant on a timeline, not a wall-clock time. Semantics are ambiguous for a date-less value, so I left them out rather than fake them. Happy to add if you want them.average— ambiguous under wraparound (naive mean of23:00and01:00gives12:00, not midnight). Left out pending a decision on desired semantics.diffFiltered/diffInHoursFiltered— these depend onCarbonPeriod-style iteration over dates; doesn't map cleanly to a time-only value.Notes
Chronos.php(DateTimeImmutable::createFromTimestampignore pattern no longer matching under PHP 8.4) is present on3.nextalready and not touched here.