2

Recently, I try to port one of our old code base, from Calendar to java.time, as we need quite a number of arithmetic functionalities, which is only found in java.time.

If we use Calendar in our current code base, we need to perform a lot of conversion back-and-forth (From Calendar to Instant, From Instant back to Calendar), in the middle of our code.

To avoid such cumbersome conversion, we decide to eliminate usage of Calendar, and port them to equivalent java.time code.

I'm a bit skeptical on my port. As compared with Calendar code, it seems to

  • Create more temporary object instances within the while loop.
  • Requires more code statements.

Calendar code

// reminderCal is Calendar object.

long startTimestamp = getStartTimestamp();
reminderCal.setTimeInMillis(startTimestamp);

while (startTimestamp <= maxTimestamp) {
    resultList.add(startTimestamp);

    reminderCal.add(Calendar.DAY_OF_MONTH, 1);

    startTimestamp = reminderCal.getTimeInMillis();
}

return resultList;

java.time code

// Epoch timestamp loopTs as initial input.

long startTimestamp = getStartTimestamp();
final ZoneId zoneId = ZoneId.systemDefault();

while (startTimestamp <= maxTimestamp) {
    resultList.add(startTimestamp);

    // More code, more temporary instances required compared
    // with Calendar's version. Not sure we're doing the right
    // way.
    Instant instant = Instant.ofEpochMilli(startTimestamp);
    LocalDateTime time = LocalDateTime.ofInstant(instant, zoneId);
    time = time.plus(1, ChronoUnit.DAYS);

    startTimestamp = time.atZone(zoneId).toInstant().toEpochMilli();
}

return resultList;

For the above code, I was wondering, are we doing the port correctly and optimized? Is there any room we can improve in our java.time's port?

Ole V.V.
  • 81,772
  • 15
  • 137
  • 161
Cheok Yan Cheng
  • 47,586
  • 132
  • 466
  • 875
  • 1
    Is there anything wrong with [`Duration.between`](https://docs.oracle.com/javase/8/docs/api/java/time/Duration.html#between-java.time.temporal.Temporal-java.time.temporal.Temporal-)? – MadProgrammer Oct 25 '18 at 05:40
  • 1
    Sorry. I don't get your point. How are we going to apply `Duration.between` for above code example? – Cheok Yan Cheng Oct 25 '18 at 05:42
  • 2
    I don't know, since I have zero context to all the extra variables in your code, but since you state you want to *"compute number of months between 2 dates"*, then `Duration.between` would be the obvious choice for the most direct solution to "that" problem – MadProgrammer Oct 25 '18 at 05:45
  • Sorry. My question isn't clear. I updated my question for better clarification, to give a better context regarding our porting. – Cheok Yan Cheng Oct 25 '18 at 05:53
  • Okay, but what is the importance of `loopTs` and `maxTs`? Could you simply add `maxTs` to an `Instant` of `loopTs`, which will give two anchor points and use something like `ChronoUnit.MONTHS.between(now, then)` (okay `Duration` doesn't have `toMonths`, my bad ) are all the "other" variables of importance? I get it down to about 5 lines without the need to involve the loops – MadProgrammer Oct 25 '18 at 05:57
  • I gave a better variable naming to the code sample, hopefully it will yields better understanding. Our purpose is to generate a list of valid timestamps in between [startTimestamp, maxTimestamp]. Hence, a while loop is required. – Cheok Yan Cheng Oct 25 '18 at 06:05
  • 1
    While I think [the answer by JB Nizet](https://stackoverflow.com/a/52982485/5772882) has addressed your concrete code example nicely, more generally you can find examples either way: some get longer, some get shorter. In my experience, the examples where the `Calendar` code is longer, it’s because code using `Calendar` tends to be rather wordy. In the case where the `java.time` code is longer, it’s because `java.time` requires you to state some things explicitly that were left to defaults before, which in turns ensures that your code more fully expresses your intentions, so it is an advantage. – Ole V.V. Oct 25 '18 at 06:31
  • Or in brief: Your decision to eliminate usage of `Calendar` is a good one. Once you learn to use java.time the best way I’m convinced that you will be happy about it. – Ole V.V. Oct 25 '18 at 07:01

1 Answers1

4

Since you want date manipulations between times in a given time zone, you shouldn't use milliseconds nor LocalDateTime, but ZonedDateTime. And I would argue that your List should contain Instants instead of longs, but let's keep it that way for now:

long startTimestamp = getStartTimestamp();
ZoneId zoneId = ZoneId.systemDefault();

ZonedDateTime maxDateTime = Instant.ofEpochMilli(maxTimestamp).atZone(zoneId);
ZonedDateTime loopDateTime = Instant.ofEpochMilli(loopTs).atZone(zoneId);

while (!loopDateTime.isAfter(maxDateTime)) {
    tsList.add(loopDateTime.toInstant().toEpochMilli());
    loopDateTime = loopDateTime.plusDays(1);
}

This is more concise, but also more readable. And all the Instant.ofEpochMilli() and toEpochMilli() calls wouldn't be needed if you wroked with Instants instead of longs.

JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • Thanks. Exactly what I'm looking for. We didn't use `Instant`, as our entire data structure is built upon primitive type. We try not to tied to selected library - Like `Calendar` or `java.time`'s. – Cheok Yan Cheng Oct 25 '18 at 06:28