Som
Som

Reputation: 1628

sonarqube error on static Calendar variable

I have the following variable declared :

private static Calendar calendar = Calendar.getInstance();

I am using that variable 'calendar' in a static method like below :

myStaticMethod(String reqDate){
    DateFormat df = new SimpleDateFormat("yyyy-MM-dd");
    Date some_date;
    long seconds = 0;
    int value = 10;
    try {
        some_date = df.parse(reqDate);
        calendar.setTime(some_date);
        calendar.add(Calendar.DATE, value);
        Date effValueDate = calendar.getTime();
        seconds = (effValueDate .getTime() - System.currentTimeMillis()) / 1000;
    } catch (ParseException e) {
        //---Do---something----
    }
}

I am getting the following sonar error :

Make "calendar" an instance variable.
Not all classes in the standard Java library were written to be thread-safe. 
Using them in a multi-threaded manner is highly likely to cause data problems or exceptions at runtime.    
This rule raises an issue when an instance of Calendar, DateFormat, 
javax.xml.xpath.XPath, or javax.xml.validation.SchemaFactory is marked static.

This is my solution, but I am not sure if the solution quality is good enough.

myStaticMethod(String reqDate) {
    Calendar calendar = Calendar.getInstance();
    // the do the next processing
}

So how can I fix the error... can I use a local Calendar variable in this case instead of class variable or is there any other smart way to address this.

Upvotes: 2

Views: 661

Answers (2)

Gautham M
Gautham M

Reputation: 4935

The sonar warning is caused because the calendar object could be updated from multiple threads. Either, as you said you could declare the calendar as local to the method.

OR 1) Make a synchronized block in your code. (using local calendar is better)

Date effValueDate ;
synchronized(calendar) {
    calendar.setTime(some_date);
    calendar.add(Calendar.DATE, value);
    effValueDate = calendar.getTime();
}
  1. RECOMMENDED: Use the new java API classes for date and time (java.time).
// or use DateTimeFormatter.ISO_LOCAL_DATE
DateTimeFormatter pattern = DateTimeFormatter.ofPattern("yyyy-MM-dd");

LocalDateTime date = LocalDate.parse(reqDate, pattern).plusDays(value).atStartOfDay();
Duration duration= Duration.between(date, LocalDateTime.now());
long seconds = duration.getSeconds();           

Read more about the new API

Upvotes: 3

Anonymous
Anonymous

Reputation: 86120

java.time

Since you seem to have accepted Gautham M’s recommendation to use java.time, the modern Java date and time API, I thought that as a supplement I would rewrite your method using java.time. I may not need to add that I wholeheartedly agree in the recommendation.

public static void myStaticMethod(String reqDate) {
    int value = 10;
    try {
        LocalDate dateThen = LocalDate.parse(reqDate).plusDays(value);
        ZonedDateTime dateTimeThen = dateThen.atStartOfDay(ZoneId.systemDefault());
        long seconds = ChronoUnit.SECONDS.between(Instant.now(), dateTimeThen);
        System.out.println(seconds);
    } catch (DateTimeParseException dtpe) {
        //---Do---something----
        System.out.println(dtpe);
    }
}

To me it appears not only simpler but also more natural to read than the original code using Calendar and the other poorly designed classes from Java 1.0 and 1.1.

I am exploiting the fact that your string format is ISO 8601 format for a date, a format that LocalDate parses as its default, that is, without any explicit formatter.

I tried this call in my time zone (Europe/Copenhagen):

    myStaticMethod("2021-06-28");

Output was:

587995

Links

Upvotes: 1

Related Questions