-3

I tried the code below.

Timestamp timestampDate = scheduled.getInterviewDateAndTime(); //from DB
Map<String, Object> map = new HashMap();    
map.put("eventTitle", "interview with");
map.put("startDateTime", timestampDate);
System.out.println("startDateTime : " + timestampDate);

long addTime = 1*60*60*1000;
timestampDate.setTime(timestampDate.getTime() + TimeUnit.HOURS.toMillis(addTime));
map.put("endDateTime", timestampDate);
System.out.println("endDateTime : " + timestampDate);

Is this the correct way or are there any good alternative approaches?

output is:

startDateTime : 2017-11-07 09:08:00.0
endDateTime : 2428-07-15 09:08:00.0

how to get correct output?

Priya
  • 21
  • 2
  • 8
  • format the code by `ctr + k` please. – W W Nov 09 '17 at 10:48
  • edited.. thank you – Priya Nov 09 '17 at 10:51
  • 2
    *Is that correct* - did you check it? – achAmháin Nov 09 '17 at 10:53
  • 1
    Whaa is the definition of that `map` ? Why first ya put into `` and then `` ? (the first line is strange) – W W Nov 09 '17 at 10:54
  • 3
    map.put("startDateTime", timestampDate); and map.put("endDateTime", timestampDate); are referencing the same object. You need to create a new object for endDateTime. – karen Nov 09 '17 at 10:55
  • Map map = new HashMap(); this is the map definition i used. – Priya Nov 09 '17 at 10:57
  • It's usually a bad idea to use `Object` in a map. Instead, it might be better to use an interface that "interview with" and timestampDate both implement. --- What is the type of timestampDate? Please don't post incomplete snippets of code. – byxor Nov 09 '17 at 11:18
  • @byxor, his is complete, the map is not related to the question and dumped here in a copy-paste accident, most likely. – M. Prokhorov Nov 09 '17 at 11:39
  • @M.Prokhorov It was incomplete at the time I posted the comment. Type information was missing (technically, it still is). – byxor Nov 09 '17 at 12:06
  • 1
    Wonder if it would be possible for you to get rid of the long outdated `Timestamp` class? The modern Java date and time API is so much nicer to work with. The `Instant` class is the natural replacement for `Timestamp`. Even if you are tied to `Timestamp`, you will still benefit from using `Instant` for the calculation of “timestamp plus 1 hour”. – Ole V.V. Nov 09 '17 at 13:00
  • Please search Stack Overflow and read the [Java Tutorials](https://docs.oracle.com/javase/tutorial/) before posting here. Your issues have been asked and answered many times already. – Basil Bourque Nov 09 '17 at 20:29

2 Answers2

1

There are several problems here:

  • java.sql.Timestamp (I assume that's what it is) is a mutable class, so setting a time on it changes the timestamp state. Even if it is not apparent with your println statements, debugging the map afterwards will reveal it in a heartbeat.
  • You logic for computing hours is wrong (you're multiplying two times there)
    1. First time when making addTime variable.
    2. Second time when using TimeUnit.toMillis()

There are (of course) several ways of fixing this:

The way I like more (it, however, requires Java 8 or ThreeTen library):

Timestamp start = obtain();
Timestamp end = Timestamp.from(start.toInstant().plus(1, ChronoUnit.HOURS));

It utilizes ability to convert a Timestamp to a java.time.Instant object (or equivalent version from ThreeTen), and then a factory constructor that will take an Instant and make a Timestamp out of it (this is Java 8 version, ThreeTen will have similar factory in other class not on Timestamp). It also utilizes a much cleaner time computation logic added in java.time compared to old datetime/calendar classes from JDK.

The second variant, which is about the same, doesn't use all the fancy stuff, but as a result is also much less readable (to me):

Timestamp start = obtain();
Timestamp end = new Timestamp(start.getTime() + TimeUnit.HOURS.toMillis(1));

As you can see, second variant is almost what you have, but without unnecessary computation.

And please don't compute these values manually; we all know that hour is supposed to be 60 minutes long and a minute is 60 second, etc., but reading this all over the place blurs eyes very quickly. You yourself has seen it by computing it manually first and then still using the TimeUnit one. And it's even worse when you need to add a day, because millis will not let you to add exactly one day without pulling a lot of information about day length at some specific point in time, considering daylight savings times and historical timezone changes. Not every minute is 60 seconds as well, there are compensative measures for those too, you know.

M. Prokhorov
  • 3,894
  • 25
  • 39
1

My recommendation for you is that you stop using the long outdated Timestamp class. Either completely if you can, or at least you minimize your use of it. I will show you code for both options. The modern Java date and time API known as java.time or JSR-310 is so much nicer to work with. And even more so when it comes to time arithmetic like adding an hour to a date-time.

java.time

Change getInterviewDateAndTime() to return an Instant. Instant is the class from java.time that naturally replaces the old Timestamp class. Also change the receiver of your Map to accept a map with Instant objects in it. Modern versions of JDBC, JPA, etc., happily retrieve Instant objects from your database and store Instants back into it.

    Instant instantStart = scheduled.getInterviewDateAndTime(); //from DB
    Map<String, Object> map = new HashMap<>();    
    map.put("eventTitle", "interview with");
    map.put("startDateTime", instantStart);
    System.out.println("startDateTime : " + instantStart);

    Instant instantEnd = instantStart.plus(1, ChronoUnit.HOURS);
    map.put("endDateTime", instantEnd);
    System.out.println("endDateTime : " + instantEnd);

Things to note: The code much more naturally and straightforward expresses the fact that one hour is added. No need for multiplications, no need for the reader to check that you multiplied the right constants, or that your multiplication didn’t overflow; it’s all taken care of. The Instant class is immutable, so there’s no risk of accidentally changing the Instant object that you have alrady added to the map.

In my example the above code printed:

startDateTime : 2017-11-29T09:15:00Z
endDateTime : 2017-11-29T10:15:00Z

Times are in UTC.

EDIT: As Basil Bourque helpfully pointed out in a comment, adding an hour can also be done this way:

    Instant instantEnd = instantStart.plus(Duration.ofHours(1));

The result is the same.

Use with legacy APIs

Assume you cannot change the return type of getInterviewDateAndTime() and/or the receiver of your map absolutely needs Timestamp objects in it. The standard way to go about such a restriction is you still use the modern API in your own code. As soon as you receive a Timestamp, you convert it to Instant. And when you need to pass a Timestamp to your legacy code, you convert your Instant only in the last moment.

    Timestamp timestampStart = scheduled.getInterviewDateAndTime(); //from DB
    Instant instantStart = timestampStart.toInstant();
    Map<String, Object> map = new HashMap<>();    
    map.put("eventTitle", "interview with");
    map.put("startDateTime", timestampStart);
    System.out.println("startDateTime : " + timestampStart);

    Instant instantEnd = instantStart.plus(1, ChronoUnit.HOURS);
    Timestamp timestampEnd = Timestamp.from(instantEnd);
    map.put("endDateTime", timestampEnd);
    System.out.println("endDateTime : " + timestampEnd);

You still have most of the advantages of the modern API mentioned above, but a couple of extra lines for the conversions. This code printed

startDateTime : 2017-11-29 10:15:00.0
endDateTime : 2017-11-29 11:15:00.0

The times are the same as above, but in my local time zone since Timestamp.toString() renders them this way (which confuses many).

Question: Can I use the modern API with my Java version?

If using at least Java 6, you can.

  • In Java 8 and later the new API comes built-in.
  • In Java 6 and 7 get the ThreeTen Backport, the backport of the new classes (ThreeTen for JSR 310). Conversion between Timestamp and Instant is a little different and goes through a class named DateTimeUtils, but it’s not more complicated. The rest is the same.
  • On Android, use the Android edition of ThreeTen Backport. It’s called ThreeTenABP, and I think that there’s a wonderful explanation in this question: How to use ThreeTenABP in Android Project.
Ole V.V.
  • 81,772
  • 15
  • 137
  • 161
  • 1
    This Answer is correct. Extract an `java.time.Instant` object from your database rather than an outmoded `java.sql.Timestamp` object. An alternative way to add an hour is with `Duration` class. `myResultSet.getObject( … , Instant.class ).plus( Duration.ofHours( 1 ) )` – Basil Bourque Nov 09 '17 at 20:26
  • @BasilBourque, while it is true that we should generally avoid using `Timestamp` in new code, the OP code might work in context of a published legacy API, and migrating to `java.time` classes might be out of the question. – M. Prokhorov Dec 27 '17 at 11:36