ppkarwasz commented on code in PR #1385:
URL: https://github.com/apache/commons-lang/pull/1385#discussion_r2094208026
##########
src/test/java/org/apache/commons/lang3/time/DateUtilsTest.java:
##########
@@ -1285,6 +1289,173 @@ public void testToCalendarWithTimeZoneNull() {
assertThrows(NullPointerException.class, () ->
DateUtils.toCalendar(date1, null));
}
+ @Test
+ void testToLocalDateTimeWithSqlDate() {
Review Comment:
Could you use a parameterized test instead of multiple test cases?
##########
src/main/java/org/apache/commons/lang3/time/DateUtils.java:
##########
@@ -1625,6 +1628,34 @@ public static Calendar toCalendar(final Date date, final
TimeZone tz) {
return c;
}
+ /**
+ * Converts a {@link Date} into a {@link LocalDateTime}, using the default
time zone.
+ * @param date the date to convert to a LocalDateTime
+ * @return the created LocalDateTime
+ * @throws NullPointerException if {@code date} is null
+ * @since 3.18
+ */
+ public static LocalDateTime toLocalDateTime(final Date date) {
+ return toLocalDateTime(date, TimeZone.getDefault());
+ }
+
+ /**
+ * Converts a {@link Date} into a {@link LocalDateTime}
+ * @param date the date to convert to a LocalDateTime
+ * @param tz the time zone of the {@code date}
+ * @return the created LocalDateTime
+ * @throws NullPointerException if {@code date} is null
+ * @since 3.18
+ */
+ public static LocalDateTime toLocalDateTime(final Date date, final
TimeZone tz) {
+ final LocalDateTime localDateTime =
LocalDateTime.ofInstant(Instant.ofEpochMilli(Objects.requireNonNull(date,
"date").getTime()),
+ Objects.requireNonNull(tz, "tz").toZoneId());
Review Comment:
_Nitpick_: Mixing `Objects.requireNonNull` with a complex expression is hard
to read.
```suggestion
Objects.requireNonNull(date, "date");
Objects.requireNonNull(tz, "tz");
final LocalDateTime localDateTime =
LocalDateTime.ofInstant(Instant.ofEpochMilli(date.getTime()), tz.toZoneId());
```
##########
src/main/java/org/apache/commons/lang3/time/DateUtils.java:
##########
@@ -1625,6 +1628,34 @@ public static Calendar toCalendar(final Date date, final
TimeZone tz) {
return c;
}
+ /**
+ * Converts a {@link Date} into a {@link LocalDateTime}, using the default
time zone.
+ * @param date the date to convert to a LocalDateTime
+ * @return the created LocalDateTime
+ * @throws NullPointerException if {@code date} is null
+ * @since 3.18
+ */
+ public static LocalDateTime toLocalDateTime(final Date date) {
+ return toLocalDateTime(date, TimeZone.getDefault());
+ }
+
+ /**
+ * Converts a {@link Date} into a {@link LocalDateTime}
+ * @param date the date to convert to a LocalDateTime
+ * @param tz the time zone of the {@code date}
+ * @return the created LocalDateTime
+ * @throws NullPointerException if {@code date} is null
+ * @since 3.18
+ */
+ public static LocalDateTime toLocalDateTime(final Date date, final
TimeZone tz) {
+ final LocalDateTime localDateTime =
LocalDateTime.ofInstant(Instant.ofEpochMilli(Objects.requireNonNull(date,
"date").getTime()),
+ Objects.requireNonNull(tz, "tz").toZoneId());
+ if (date instanceof java.sql.Timestamp) {
+ return localDateTime.withNano(((Timestamp) date).getNanos());
+ }
Review Comment:
Currently, Commons Lang only depends on the `java.base` and `java.desktop`
modules. Introducing a direct usage of `java.sql.Timestamp` would add a new
dependency on the `java.sql` module, which we want to avoid.
Could you access both the `java.sql.Timestamp` class and its `getNanos()`
method via reflection instead? This approach avoids adding the dependency while
still enabling the required functionality.
For reference, you can see an efficient example of handling `java.sql` types
through reflection in
[`org.apache.logging.log4j.util.StringBuilders`](https://github.com/apache/logging-log4j2/blob/2.x/log4j-api/src/main/java/org/apache/logging/log4j/util/StringBuilders.java).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
