0

I have a Calendar object that is being passed to indicate timezone when I insert a row in the database:

private long insertTime() {
        statement.setTimestamp(index, new java.sql.Timestamp(Instant.now().toEpochMilli()),
                Calendar.getInstance(TimeZone.getTimeZone(TIME_ZONE)));
}

I was suggested to create private static calendar object in the class and use it:

private static final Calendar = Calendar.getInstance(TimeZone.getTimeZone(TIME_ZONE));
...
...
...
private long insertTime() {
        statement.setTimestamp(index, new java.sql.Timestamp(Instant.now().toEpochMilli()), calendar);
}

Both code works so I am not sure what is the value of changing my code to second one. Only possible value I can see is that this code runs quite often (multiple times in minutes), so instantiating calendar object multiple times is not good practice. But does it make a lot of difference?

EDIT: this calendar object is only used for this method once.

Jonathan Hagen
  • 580
  • 1
  • 6
  • 29
  • is there any chance `insertTime` could be called from different threads? – Joni Aug 28 '20 at 15:36
  • 1
    statically caching objects which are safe to use with multiple threads will indeed provide a benefit. however, Calendar is _not_ safe to use with multiple threads and therefore _should not_ be held statically. – jtahlborn Aug 28 '20 at 15:37
  • So it is not being used anywhere except here, one place. Only think I can think of is that this code runs multiple times in a second. – Jonathan Hagen Aug 28 '20 at 15:43
  • For thread safety, it doesn't matter if this is only place where the object is used. What matters is if this method can be called by different threads. This is a lot more important than how many times per second the method is called. – Joni Aug 28 '20 at 16:21
  • I think these opinions on thread safety are incorrect. The object is safely published by using the `final` keyword. ( https://www.javamex.com/tutorials/synchronization_final.shtml and also Brian Goetz's *Java Concurrency in Practice.*) As long as it is not mutated it is thread safe. (If `setTimeStamp` does mutate the object then your point is correct, but is that a actually the case?) @Joni – markspace Aug 28 '20 at 17:10
  • 1
    @markspace yes the calendar is actually mutated within `setTimestamp` – Joni Aug 28 '20 at 17:25
  • Not what you asked, but since JDBC 4.2 don’t use `Timestamp`, `TimeZone` nor `Calendar`. Instead pass either an `OffsetDateTime`, an `Instant` or a `LocalDateTime` to `statement.setObject()`. – Ole V.V. Aug 31 '20 at 06:06

1 Answers1

0

From PreparedStatement#setTimestamp(int,java.sql.Timestamp,java.util.Calendar) (emphasis mine)):

Sets the designated parameter to the given java.sql.Timestamp value, using the given Calendar object. The driver uses the Calendar object to construct an SQL TIMESTAMP value, which the driver then sends to the database. With a Calendar object, the driver can calculate the timestamp taking into account a custom timezone. If no Calendar object is specified, the driver uses the default timezone, which is that of the virtual machine running the application.

This means that the method will use the Calendar object. If your method can be executed by multiple threads at the same time i.e. running in a web application, then this would generate an issue (specifically, a race condition) at runtime.

Take into account that it doesn't matter if the code is in a single place. For cases like this, it is better to think how this behaves at runtime: is the code executed by two threads at the same time? If yes, it's better to create instances of mutable objects per method execution.

The thread safe option would be to create the Calendar instance per method execution.

Luiggi Mendoza
  • 85,076
  • 16
  • 154
  • 332
  • I think you and jtalhborn need to justify this opinion further. The `static final` object is safely published. (C.f. https://www.javamex.com/tutorials/synchronization_final.shtml ) As long as the object is not mutated it is thread safe. Can you point out where this object is mutated later? – markspace Aug 28 '20 at 17:08