Comments (13)
Java does not have objects for Second, Minute, Hour, Day AFAIK. They've been explicitly introducing classes for just a handful of them: Year, Month, DayOfWeek.
The LocalDateTime object returns just two of them as non-integers: getMonth()
and getDayOfWeek()
.
It may make sense as @joshdifabio explained very well:
We don't refer to month 7 or weekday 3, we say July and Tuesday, whereas we do refer to years and days of the month in numeric terms.
I have a little problem with getMonth()
myself for the reasons explained above, but for getDayOfWeek()
, it makes a lot of sense as there are several conventions, such as using 0
or 7
for Sunday. Returning an object removes all confusion.
from date-time.
Actually, I may go straight to changing the signature in 0.7.0
. Either way it will throw an error if people didn't address the deprecation, so there may be little value in doing it in 2 steps. Thoughts?
from date-time.
You're right that these are inconsistencies in the API. I found the decisions quite hard to make at the time I wrote these.
As I see it now, I'd say that these should all return integers. An object can be explicitly requested by calling, for example, DayOfWeek::of($date->getDayOfWeek())
if needed.
What do you think?
And do you see other inconsistencies?
from date-time.
I found the decisions quite hard to make at the time I wrote these.
I can imagine! This really is an excellent package though and it's clear that a lot of time, thought and skill has gone into it -- thank you so much for taking the time to create it and open source it.
Regarding the matter at hand, the first thing I'd say is that we can't possibly replicate the collective brainpower and empirical evidence that went into designing JSR 310 and so, where practical, I would suggest simply copying JSR 310's API. I think we are unlikely to make better subjective judgements than the original authors made, particularly because we have far less experience with these APIs than the authors of JSR 310 had (JSR 310 is primarily a formalisation of an API which was used by thousands of developers over a number of years before JSR 310 came along, as you likely already know).
Regarding java.time's API, values which are Enums are primarily passed around as Enums (Month
and DayOfWeek
), not scalars. So, all of the getMonth()
and getDayOfWeek()
methods return Enums, not ints. I would simply replicate this here. getYear()
and getDayOfMonth()
are different because these are not Enums.
In terms of practical benefits, and my personal opinion, passing integers around means we have to constantly validate values to check whether they are valid months/weekdays, which negates one of the benefits of this package. Value objects and enums can eliminate a lot of boilerplate. Therefore, the enums are more useful than bare integers, and so I would primarily use them where they're available. Furthermore, months and weekdays are not naturally numeric. We don't refer to month 7 or weekday 3, we say July and Tuesday, whereas we do refer to years and days of the month in numeric terms.
However, I would very much put my opinions as secondary to those of the original authors of JSR 310, which just happen to be the same in this instance.
And do you see other inconsistencies?
I haven't noticed anything else yet. Fantastic work, thank you again for using your time and skill to serve the open source community.
from date-time.
Regarding the matter at hand, the first thing I'd say is that we can't possibly replicate the collective brainpower and empirical evidence that went into designing JSR 310
This is very true, however PHP does not have the same functionalities as Java, so small differences are to be expected in some places. Also, for the record, I'm not against making deliberate opposite decisions, I did this for example in brick/money: JSR 354 has been a big source of inspiration and I took quite a lot of time to browse the spec and the reference implementation, but then I deliberately chose to not embed things such as rounding mode into the Money itself. Who made the best call? I still don't know, but I'm happy with the end result now, and the feedback I got so far was positive.
Furthermore, months and weekdays are not naturally numeric. We don't refer to month 7 or weekday 3, we say July and Tuesday, whereas we do refer to years and days of the month in numeric terms.
This is a very good point! 👍
I guess I remember what felt inconsistent to me at the time (and still does, to some extent), is the following:
- a LocalTime has 3 fields (ignore the nanos for now): hour, minutes and seconds
getHour() : int
getMinute() : int
getSecond() : int
- a LocalDate has three fields: year, month and day:
getYear() : int
getMonth() : Month
getDay() : int
It feels a bit wrong to me, that most fields are returned as integers, and one of them is returned as an enum (well, an object, as we don't have enums).
You could be surprised that you can't do:
sprintf('...', $date->getYear(), $date->getMonth(), $date->getDay());
Actually I can see that Java provides a getMonthValue()
method, which fits the bill. As a consumer of the API though, I think it can be surprising to not get the integer value out of the getMonth()
method, and have to use getMonthValue()
instead.
This is just my gut feeling, of course, and you approach is most likely the correct one.
I would be happy to revisit the API for version 0.2; would you be willing to compare Brick\DateTime's API with Java's, and list all differences here?
At least we could see where exactly the APIs diverge, and this could help orient the decisions a lot.
from date-time.
I personaly prefer to have simple VO object types even when there is no logic on given object. It is then type-safe to pass them around. Question is - is there usecase for passing just Month around? If it is with year there is YearMonth
object for that. And that makes much more sense in public API.
I'm not quite sure if it can be practical to have Second
, Minute
, Hour
, Day
, Month
, Year
as it will be quite messy to create these objects then. However they are not just an integers. They must be from domain of numbers - e.g. seconds must be 0-59
, hour 0-23
. Any ideas how to make API practical to use with these new objects?
from date-time.
I think at this point it's all pretty subjective so I won't attempt to add anything else to the discussion beyond what I've already said.
I would be happy to revisit the API for version 0.2; would you be willing to compare Brick\DateTime's API with Java's, and list all differences here?
I don't think I will be able to commit the time to do this, sorry :(.
from date-time.
I'm still hesitating to change getMonth()
to return a Month
, I'm trying to find a use case where this would be desirable, and weighting this against everyday use of an int
for the Month.
For example, this code from the tests:
$this->compare([$year, $month, $day], [
$date->getYear(),
$date->getMonth(),
$date->getDay()
]);
Would become:
$this->compare([$year, $month, $day], [
$date->getYear(),
$date->getMonthValue(),
$date->getDay()
]);
Which I find quite counter-intuitive.
And because we don't have enums in PHP, Month is a class instance, so you can't compare the value to constants directly. For example, in Java you would do:
if (date.getMonth() == Month.JANUARY) {
Where in PHP this becomes more verbose if we decide to return an object:
if ($date->getMonth()->is(Month::JANUARY)) {
Currently, we can do:
if ($date->getMonth() === Month::JANUARY) {
Which is cleaner IMO. At the same time, I agree that there is somewhat of an inconsistency to return DayOfWeek
as an object, and month as an int
.
I would be interested to hear a valid use case for using LocalDate::getMonth() : Month
.
from date-time.
Actually there is no getDay()
in Java, it's called getDayOfMonth()
. So if we mimic their API, the example above would become:
$this->compare([$year, $month, $day], [
$date->getYear(),
$date->getMonthValue(),
$date->getDayOfMonth()
]);
Which is harder to read and memorize.
from date-time.
For example, this code from the tests:
$this->compare([$year, $month, $day], [
$date->getYear(),
$date->getMonth(),
$date->getDay()
]);
I don't think this is a good example, because you would not typically hold separate references to a year, month and day of month if you were using this library properly; rather, you would use a LocalDate
.
Month
is useful when you are dealing with only a month, otherwise you would use LocalDate
, YearMonth
or MonthDay
, so it would be best if we constrain our discussion to such cases. In these cases where we are dealing with only a month, we probably either have a function parameter or return type which is a value representing a month; depending on preference, the type of this value could be Month
or int
. If Month
, we don't need to validate such values. Assuming we want to fail fast on errors, which we should, the month in int
form potentially needs to be validated every time we see it.
function foo(Month $month) {
// we know that $month is valid
}
function foo(int $month) {
// we do not know that $month is valid
}
And because we don't have enums in PHP, Month is a class instance, so you can't compare the value to constants directly.
Enums are objects in Java too. Often these enum objects are constructed like this in PHP: Month::january()
or Month::JANUARY()
.
Actually there is no getDay() in Java, it's called getDayOfMonth() ... Which is harder to read and memorize.
This is another case where you should have stuck with the Java API in my opinion. getDay
is ambiguous. Sunday is also a day. For example, in JavaScript, getDay()
refers to the day of week (MDN).
A lot of very smart people have already had these conversations.
from date-time.
I'll revisit this issue soon, in the light of enums now being available in PHP 8.1.
from date-time.
@BenMorel I think this issue can now be closed, what do you think ?
from date-time.
@gnutix I think we can close this once getMonth()
will return a Month
enum.
As I see it:
- deprecation of
getMonth(): int
in favour ofgetMonthValue()
in0.6.0
(✅ done) - removal of
getMonth(): int
in0.7.0
- addition of
getMonth(): Month
in0.8.0
from date-time.
Related Issues (20)
- `ZonedDateTime::of()` issue when year is less than 1892 HOT 5
- Introduce a ZonedDateTimeRange HOT 10
- ZonedDateTime::toDateTime and getDateTime conflict HOT 3
- Add LocalDateRange::toPeriod HOT 4
- Named Constructors for TimeZone HOT 2
- isEqualTo(TimeZone::utc()) is not normalized HOT 7
- `Interval` is so barebones compared to `LocalDateRange`
- Add Seconds value object HOT 3
- Interface for any object representing an interval of time HOT 8
- Some objects with ISO representations are missing a parse method
- LocalDateRange / endExclusive HOT 6
- Negative Duration HOT 2
- `null`-friendly methods HOT 13
- [performance] Optimizing LocalDate::plusDays() method HOT 1
- Performance optimization: skip validation when creating objects (SOLUTION: use Reflection) HOT 8
- Broken badge on README HOT 2
- Year::toISOString() not conformation to ISO 8601 HOT 2
- Add `DefaultClock::travelForwards(Duration)` HOT 4
- `LocalDate::parse(...)` does not recognise ISO-8601 HOT 1
- Year::atMonth() should accept a Month HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from date-time.