[Clock] Add DatePoint: an immutable DateTime implementation with stricter error handling and return types
#51415
Conversation
Nice. 🙂
That's my biggest fear about adding such a class, tbh. If by "more methods" we mean polyfills for features that will be added in future PHP versions: yes, totally. But please, let's not open this class up for all kinds of utility methods. 🙈 Oh, and please add tests for that new class. 🙏🏻 |
We don't want to create a new Carbon class. |
|
Have you looked at the Brick DateTime library? Its API looks very good and has many types for many cases. Much more stricter and more powerful than Carbon. |
My point exactly. |
d158df1 to
0dd8cfa
Compare
|
PR ready, with tests !
Same here, I stroke the line in the PR description. Guarding this is why we have a core-team :)
I didn't, but we just ruled such approaches out so 🤷 |
|
Next steps to consider:
But before, votes pending @symfony/merger 8000 s ;) |
|
I think it's not a good idea to name this class similar to one from PHP's stdlib that has different behavior. You'll have to scroll up to the use statements to know if The only case where this is desired is our polyfill packages, which provide polyfills for stdlib features. Eventhough all behavior changes in this class are probably the desired behavior for |
|
|
To me, PHP should have only offered a But I don't think this is the job of a framework / component to do that, as the class name is identical but for a totally different and breaking behavior. Sure you explicity import it, but people will need to check twice if this |
70cce4e to
6bd09cf
Compare
|
Figuring out the namespace requires a basic IDE and hovering the symbol, no need to scroll up. For ppl that don't use
I'm not sure to get what you mean here. The proposed class is not a polyfill. It's a stricter DateTime. We don't need any agreement from php-core to be future proof, thanks to LSP. Still, if we want another name, I'd propose function now(string $modifier = 'now')
{
return new Now($modifier);
} |
|
Not a fan of public function getLastUpdated(): Now |
|
|
|
|
|
DateMark |
|
To be honest, since typing my previous message I'm a bit on the fence on whether we should want this feature at all. I don't feel comfortable with a framework that "fixes"/"patches" the standard library. Of course, this is only one class, hence I'm not 100% against it, but if we accept this, would we also accept e.g. adding new namespaced functions that introduce strict error handling to stdlib functions that currently return false on error? (i.e. https://github.com/azjezz/psl) And if we don't (I hope we don't!), why would we accept this? This is a language issue that should be solved on language level imho. Create a new date time object (you can directly properly namespace it) and make it behave like it should with our current experience. In a major version or two, deprecate the current date time objects and we're done. PHP has managed much bigger API BC breaks like this (e.g. |
|
The core motivation for adding this class on my side is to be able to create date-time instances that leverage the global static clock. Aka to make One could say that the function foo(DateTime $date = new DateTime) {...} |
|
Thanks for the explanation. While using Cronos, I haven't found the need to I'm now learning towards reducing this class to exactly this use-case: being the class Now extends \DateTimeImmutable
{
public function __construct()
{
$now = Clock::get()->now();
parent::__construct($now->format('c'), $now->getTimezone());
}
}This does not broaden the scope of the Clock component and keeps us far away from inventing our own language :) |
e3767db to
0dc449a
Compare
|
|
2d1f62a to
7a5db60
Compare
|
But we can also call it |
7a5db60 to
1b755c6
Compare
TimePoint: an immutable DateTime implementation with stricter error handling and return typesDatePoint: an immutable DateTime implementation with stricter error handling and return types
1b755c6 to
8315f72
Compare
|
Updated to I like that typing |
|
That makes sense 👍🏻😃 |
…ricter error handling and return types
8315f72 to
2f384d1
Compare
|
Thank you @nicolas-grekas. |
The native
DateTime/Immutableclasses have some legacy that we can get rid of by creating a child class.Here comes
Symfony\Component\Clock\DatePoint, which extends the nativeDateTimeImmutableand provides the following advantages:new DatePointis compatible withClock::set())This also opens the possibility to add more methods to this<- we don't want that :)DatePointclass in the future if we want to.