Skip to content
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

RFC Datetime and Date class name #71

Closed
dereuromark opened this issue Jan 4, 2016 · 7 comments
Closed

RFC Datetime and Date class name #71

dereuromark opened this issue Jan 4, 2016 · 7 comments
Milestone

Comments

@dereuromark
Copy link
Member

The more I use it and think about it the more sense it would make to have a consistent naming scheme for the class names.

//Currently
$time = new Chronos('2015-10-21 16:29:00');
$date = new Date('2015-10-21');
$time = new MutableDateTime('2015-10-21 16:29:00');
$date = new MutableDate('2015-10-21');

// Prosposed
$time = new DateTime('2015-10-21 16:29:00');
$date = new Date('2015-10-21');
$time = new MutableDateTime('2015-10-21 16:29:00');
$date = new MutableDate('2015-10-21');

Afaik the only reason was namespace clashing/confusion.
But with all the other 3 classes around I think the confusion is rather internal now, with Chronos not just as namespace but also as class name sticking out here.
Chronos also does not say anything about the type of date it contains or can work on ( is it Date, or DateTime or just Time or ...). For me that is a cool package name, but not a very explaning class name.
Thus I think DateTime would indeed be the more sane name for the Chronos class IMO.

We can change it now before releasing a 1.0.0.

@dereuromark dereuromark added this to the 0.5.0 milestone Jan 4, 2016
@lorenzo
Copy link
Member

lorenzo commented Jan 4, 2016

That could potentially be painful for end users, since using both \DateTime and Chronos\DateTime in the same class would require aliasing.

@dereuromark
Copy link
Member Author

And the other 3 don't have a core PHP counterpart you mean?

One could probably argue that using both in the same class file is usually a not a good idea. Or should make use of some aliasing like

use DateTime as PhpDateTime;

or manually

new \DateTime()

etc. Since the goal of using this Chronos DateTime class is to make things more consistent you should be using it everywhere to be able to leverage that. Using the PHP core one on top in the same files should be the exception to the rule in that case.

But I understand what you mean.

@lorenzo
Copy link
Member

lorenzo commented Jan 4, 2016

Yeah, that's what I mean... In general I don't like class names to be repeated, even if we have namespaces, because reading the code becomes more difficult

@dereuromark
Copy link
Member Author

In that case pre- or suffixing is usually the way to go, but ChronosDateTime could become a little bit much to write maybe?
I still think, though, that mixing the classes inside a class is an edge case. As soon as testing, timezones or buggy core behavior is involved that will blow up eventually.

@josegonzalez
Copy link
Member

What about:

class Chronos extends Chronos\DateTime

That way users that want to just use the Chronos version can, and all others can use the Chronos name.

@dereuromark
Copy link
Member Author

That sounds like a fair (and BC) compromise.

@antograssiot
Copy link
Contributor

PR #74 is open so discution can take place there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants