0

I'm working with some legacy code and I'm iterating through some values, the legacy code is checking for the start of a new day using an sql.timestamp value:

public static final long MILLIS_PER_SECOND = 1000;
public static final long MILLIS_PER_MINUTE = 60 * MILLIS_PER_SECOND;
public static final long MILLIS_PER_HOUR = 60 * MILLIS_PER_MINUTE;
public static final long MILLIS_PER_DAY = 24 * MILLIS_PER_HOUR;

 if (entry.getPeriodEnd().getTime() % TimestampUtils.MILLIS_PER_DAY == 0 || i >= throughputEntries.size()) {

       ...
}

The result is never 0 so it only goes into the if when i >= throughputEntries.size()

I can't get me head around how he was getting a 0 result, possibly his data was different and always ended with a certain time period that would produce the 0

I could rewrite the code maybe to do a check on the date, but would like to know how he achieved it with his code.

I'm not sure that this could be solved with the code I have provided, but maybe I'm missing something...

Edit

So I got my head around this in the end, thanks to @t0r0X & @Leo

I've modified my code to use Joda-Time as follows:

 LocalDate newDay = null;
 int count = 0;


public void calcAvg(){

    DateTimeZone zone = DateTimeZone.getDefault();
    LocalDate throughtputDate = entry.getPeriodEnd().toDateTime(zone).toLocalDate();

    if ( this.newDay(throughtputDate) || i >= throughputEntries.size()) {

       ...
    }

}


public boolean newDay(LocalDate date){

    boolean go = false;
    if(count !=0){
        if(date.isAfter(newDay)){

            newDay = date;
            go = true;
        }
    }
    else{
        newDay = date;
        count++;
    }

    return go;
}

Probably a cleaner way of checking for the new date than the method I've written, but time waits for no man.

Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
JTK
  • 1,469
  • 2
  • 22
  • 39
  • 2
    are you using joda or java8? some old java date functions are quite buggy (sometimes 1 hours "vanishes" just because some daylight savings takes over, things like that. As you can imagine, a day is not always 60*60*24 seconds a day. (think about leap years for example) – Leo Jan 25 '16 at 19:12
  • I'm in the process of re-factoring the code to use Joda, I have done this for a lot of the code, but the part is still using java8, I want to get it working before I refactor it. – JTK Jan 25 '16 at 19:13
  • 3
    if you're using java8, you don't need joda because JSR-310 implemented on java8 was highly inspired on joda (hence you can use java.time) – Leo Jan 25 '16 at 19:16
  • 1
    In your shoes, if what you have are millis, you must create new dates using them (new Date(millis)) and then use Date arithmetic on them. Do not assume a day is a fixed number of millis (from a Calendar perspective, it is not) – Leo Jan 25 '16 at 19:19
  • Thanks for your help @Leo :) – JTK Jan 25 '16 at 20:32
  • @Johntk Your Question is not clear. Your example code refers to variables that are not explained. Can you simplify? What exactly go you get, and what exactly do you want? Are you testing to see if a date-time has the same date as "today"? – Basil Bourque Jan 26 '16 at 01:18
  • The code was iterating through values pulled form a DB and adding them up for a daily total, the if statement in the first snippet of code was calculating if the date of the next variable pulled from the database was in a new day and if it was it entered the if, I couldn't get it to work or understand how it could have worked, I do now as the other commenters have pointed out and I have solved the problem using Joda time. I hope that answers any questions, I can modify the question more if you'd like me to be more clear on anything? – JTK Jan 26 '16 at 23:08

2 Answers2

3

The author of the original code may have probably used synthetic "round" data. Maybe using the (nowadays deprecated) java.sql.Timestamp constructor ...

new Timestamp(116, 1, 25, 0, 0, 0, 0)

or maybe using the (also deprecated) Date constructor ...

new Timestamp(new Date(116, 1, 25).getTime())

Or he might have parsed some test data like e.g. '2016-01-25'...

Look ma, no millis! ;-)

Anyway, to check for a "new day" depends on how you define that. Usually you need a reference date, like the previously processed entry.

Update:

Looking a 4th time at the code entry.getPeriodEnd().getTime(): the entries look like time periods... So there must be a getPeriodBegin() method... and I suppose the needed check is to verify if the period end is in another day than the period begin... Am I right?

Dirty code (deprecated methods):

Timestamp begin = entry.getPeriodBegin()
Timestamp end = entry.getPeriodEnd()
if (begin.getYear() != end.getYear() || begin.getMonth() != end.getMonth() || begin.getDay() != end.getDay() ....) 

Clean code:
My recommendation: don't even start with java.util.Calendar (proposal on Stackoverflow). Either use DateUtils from Apache Commons Lang (also suggested here on Stackoverflow; comments are relevant), or, my favourite, get JodaTime and stop worring (suggestion also on Stackoverflow; comments are relevant). I personally have always got happy with JodaTime.

PS

Not to forget (thanks to @Leo for his comment on the question, I'm still living, ahem, working, in a Java 6-7 world :-/ ...): if using Java 8 or later, go for the new java.time classes.

Community
  • 1
  • 1
t0r0X
  • 4,212
  • 1
  • 38
  • 34
  • I've seen this used in the code: `new Timestamp(new Date(116, 1, 25).getTime())` I'll have to go look for it, but it's not used in `TimestampUtils` – JTK Jan 25 '16 at 19:34
  • "Anyway, to check for a "new day" depends on how you define that. Usually you need a reference date, like the previously processed entry." So my best bet is to change the code and check for a new day using a previous date as a check. – JTK Jan 25 '16 at 19:38
  • 1
    @Johntk yes, but before thaat please validate my "extended" interpretation of your code snippet – t0r0X Jan 25 '16 at 19:57
  • Yes you're 100% they are time periods, sorry I should have been more clear on that, and yes he has a `Timestamp begin = entry.getRetrived()` `Timestamp end = entry.getPeriodEnd()` I've pasted in the rest of the if so you can see what he's done – JTK Jan 25 '16 at 20:09
  • I've started using joda elsewhere in the code, so I'll take your advice and start reading up on that link now, thanks for your help with this. – JTK Jan 25 '16 at 20:13
  • 2
    @Johntk hope it helps. As @Leo mentioned in his comment on your question, starting with Java 8 you should use `java.time`, it's JodaTime ported to the Java runtime. – t0r0X Jan 25 '16 at 20:25
  • I might have to do more refactoring than it's worth to start using java8 now, hindsight a bitch, I'm on a time limit to get this working and the legacy code is making progress painfully slow. I'll make sure to thank @leo for his help as well, cheers man. – JTK Jan 25 '16 at 20:30
1

The Answer by t0r0X is correct, but uses outdated classes.

Also, you should generally not be doing business logic using java.sql.* classes. They are intended for data transit in and out of your database but nothing more. Immediately convert java.sql.Timestamp/.Date/.Time to java.time types. Perform your business logic with java.time types. Hopefully someday the java.sql types will fade away after JDBC drivers have been updated to handle java.time directly.

The Question and comments are not clear. But I'll take a stab it as it seems to have to do with accumulating numbers by date.

java.time

Java 8 and later comes with the excellent java.time framework. See Tutorial. These new classes supplant the old troublesome classes of java.util.Date/.Calendar.

Among these new classes is LocalDate for a date-only value without time-of-day nor time zone. The java.sql.Timestamp is a date-time value rather than date-only. But we can convert to java.time.Instant, a moment on the timeline in UTC. Then we apply a time zone (ZoneId) to get a ZonedDateTime. From that we may extract a LocalDate. That LocalDate is just what we need as keys for a Map to accumulate a sum of numbers.

Instant instant = myJavaSqlTimestamp.toInstant();
ZoneId zoneId = ZoneId.of( "America/Montreal" );
ZonedDateTime zdt = ZonedDateTime.ofInstant( instant , zoneId );
LocalDate localDate = zdt.toLocalDate();

Note that time zone is crucial here. While LocalDate retains no time zone, a date only has meaning in the context of a time zone. The date is not the same around the world for any given moment. If not specified, your JVM’s current default time zone is applied. Other code on this page explicitly asked for the current default. I recommend strongly against using the default in this problem. The JVM’s current default time zone can change at any moment before the app runs and even while your app runs! If totaling numbers by date, the business logic must have some time zone in mind for the meaning of that date. Specify that time zone explicitly. Here I have arbitrarily chosen America/Montreal.

Earlier you would have instantiated a Map of LocalDate to a sum of the numbers being collected. I will assume we are dealing with BigInteger as our number type for this demo.

Map< LocalDate , BigInteger ) map = new HashMap<>();

We add an entry into that Map for each LocalDate date value we encounter. We assume we have an BigInteger myNumber obtained from database via JDBC.

BigInteger oldTotalForDate = map.get( localDate );
BigInteger newTotalForDate = ( null == oldTotalForDate ) ? myNumber : oldTotalForDate.add( myNumber) ;
if ( null == newTotalForDate ) {  // If we failed to get new sum.
    // TODO: Handle error condition. Perhaps: map.remove(key);
} else {  // Else normal, we have a new total. Store it in map.
    map.put( localDate , newTotalForDate );  // Replaces any old value.
}

That code above may be shortened by using the new Map::merge method using new Lambda syntax features in Java 8 and later.

Community
  • 1
  • 1
Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154