-
Notifications
You must be signed in to change notification settings - Fork 1.7k
DateTime should have a copy constructor with value overwrite #24644
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
Comments
This could be a |
The time zone issue is really vital, though. Creating the "same" time in a different time zone, invisibly, would be really bad. I don't think this can be put in until we have clear agreement on what the time zone and utc behavior should be, after thinking it through carefully. |
The version in the pull request also includes the isUTC flag. |
An internal Flutter customer (customer: fast) had an issue with their shipping app last week due to using |
The I think a solution could be to have a proper |
We initially had different If we wanted to be correct, then I still think that having a simpler class in the core libraries is a good idea (although I would love to see a complete date library), but we should make it easier to avoid mistakes. Note that the internal Google3 libraries provide a |
Java solves this problem by providing the |
While the final decision does not come, I've created this extension to help:
PS it works perfect with |
Extensions can work. I'd prefer something like interface default methods since they'd allow being virtual, and feels unnecessarily complicated to declare extension methods in the same library as the base type - but it's breaking to add methods to interfaces without interface default methods. |
This extension can now be simplified quite a bit with constructor tear-offs (Dart 2.17): extension DateTimeExtensions on DateTime {
DateTime copyWith({
int? year,
int? month,
int? day,
int? hour,
int? minute,
int? second,
int? millisecond,
int? microsecond,
bool? isUtc,
}) {
return ((isUtc ?? this.isUtc) ? DateTime.utc : DateTime.new)(
year ?? this.year,
month ?? this.month,
day ?? this.day,
hour ?? this.hour,
minute ?? this.minute,
second ?? this.second,
millisecond ?? this.millisecond,
microsecond ?? this.microsecond,
);
}
} |
For folks who still have to write on Dart SDK < 2.15 extension DateTimeExtension on DateTime {
DateTime copy() {
if (isUtc) {
return DateTime.utc(
year,
month,
day,
hour,
minute,
second,
millisecond,
);
}
return DateTime(
year,
month,
day,
hour,
minute,
second,
millisecond,
);
}
DateTime copyWith({
int? year,
int? month,
int? day,
int? hour,
int? minute,
int? second,
int? millisecond,
bool? isUtc,
}) {
if (isUtc ?? this.isUtc) {
return DateTime.utc(
year ?? this.year,
month ?? this.month,
day ?? this.day,
hour ?? this.hour,
minute ?? this.minute,
second ?? this.second,
millisecond ?? this.millisecond,
);
}
return DateTime(
year ?? this.year,
month ?? this.month,
day ?? this.day,
hour ?? this.hour,
minute ?? this.minute,
second ?? this.second,
millisecond ?? this.millisecond,
);
}
} |
Could we include |
We could make it an extension method, then it's not as breaking. |
Is it not worth adding it as a breaking change though? It has been discussed for like 8 years now. |
If it would be added as an extension, could it be in the same file as the |
I put up a draft PR, but maybe it is not needed now if #49928 will lead to that it is implemented as an instance member instead. |
I don't think adding a Most of the time when people would want to use extension on DateTime {
// This might land you at a time more than an hour in the past in the second duplicated interval of a time change
DateTime floorToSecond() => copyWith(millisecond: 0, microsecond: 0);
// The day might not have a time of day the same as `this` if the time got put forward that day.
// A better method should probably have a nullable return type to alert users to that possibility.
DateTime sameTimeDifferentDay(DateTime day) =>
copyWith(year: day.year, month: day.month, day: day.day);
// Similar problem to sameTimeDifferentDay with times but also with February 29.
// If `this` is in a leap year on February 29 and `year` is not a leap year,
// then the result will be on March 1. This may or may not be what the user wants.
// Again the return type should probably be nullable.
DateTime inDifferentYear(int year) => copyWith(year: year);
// Correct but verbose. User might miss microsecond when implementing like for example in the `copyWith` example implementations above AND IN THE PATCH!!!
DateTime get startOfDay =>
copyWith(hour: 0, minute: 0, second: 0, millisecond: 0, microsecond: 0);
} Point is, each of these use cases has their own problems and requires their own solutions. So if anything, i think more high level methods should be added to |
I'll say it here in a separate comment again for visibility: |
I think this is a trade-off that is worth taking, the users are already creating their own
Creating high level methods for
Good catch, I'll add that! |
The comment about It's something we can consider worrying about and designing for. It's not clear what the perfect solution is, because it very much depends on the intent of the caller. As stated, if you do We could try to do both: Use the verbatim result and the "adjust-for-time-difference" result, and if they end up in the same time zone, it's fine. If not, we need to figure out which one to use. Or we can just tell people that using |
Since I don't think there is any perfect solution this is the best option imho. |
This change has been discussed for 8+ years and it would of course be preferred to be added as an instance method, but since that is a breaking change I added it as an extension as discussed here: #24644 (comment) Change-Id: Iebb9f300e449920ae8891abac88f30b271321661 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/258541 Commit-Queue: Lasse Nielsen <[email protected]> Reviewed-by: Nate Bosch <[email protected]> Reviewed-by: Lasse Nielsen <[email protected]>
This is now merged and will be available from Dart 2.19! 🥳 |
There is a copyWith method available in the upstream since Dart 2.19: dart-lang/sdk#24644
There is a copyWith method available in the upstream since Dart 2.19: dart-lang/sdk#24644
I often have to create new DateTime object that are based on other DateTime objects.
There are some methods like subtract and add which can create new instances, but sometimes you just want to have a new DateTime object at the same time but on a specific date or on the same day on another time.
For that I have created a small helper:
It would be really handy if there would be for example a constructor or a method on DateTime that could provide this.
Even better would be to keep the same timezone, but it's not a big deal for me, so I have not implemented in my solution yet.
The text was updated successfully, but these errors were encountered: