Class to transform milliseconds to current date

Question

The task Given a variable of type long with the number of milliseconds from January 1, 1970 until a later date (CurrentDate). Build a class for get the year, month, day of year, day of month, hour, minute and second of the CurrentDate.

Solution 1:

public class MilisecondToDate {

    public final long MILISECONDS_PER_YEAR = 31536000000L;
    public final long MILISECONDS_PER_DAY = 86400000L;
    public final long MILISECONDS_PER_HOUR = 3600000L;
    public final long MILISECONDS_PER_MINUTE = 60000L;
    public final long MILISECONDS_PER_SECONDS = 1000L;

    public final int FIRST_YEAR = 1970;

    public final int DAY_OF_YEAR_IN_END_OF_JANUARY = 31;
    public final int DAY_OF_YEAR_IN_END_OF_FEBRUARY = 59;
    public final int DAY_OF_YEAR_IN_END_OF_MARCH = 90;
    public final int DAY_OF_YEAR_IN_END_OF_APRIL = 120;
    public final int DAY_OF_YEAR_IN_END_OF_MAY = 151;
    public final int DAY_OF_YEAR_IN_END_OF_JUNE = 181;
    public final int DAY_OF_YEAR_IN_END_OF_JULY = 212;
    public final int DAY_OF_YEAR_IN_END_OF_AUGUST = 243;
    public final int DAY_OF_YEAR_IN_END_OF_SEPTEMBER = 273;
    public final int DAY_OF_YEAR_IN_END_OF_OCTOBER = 304;
    public final int DAY_OF_YEAR_IN_END_OF_NOVEMBER = 334;
    public final int DAY_OF_YEAR_IN_END_OF_DECEMBER = 365;

    private long milisecond;

    public MilisecondToDate(long milisecond) {
        this.milisecond = milisecond;
    }

    public int getYear() {
        return FIRST_YEAR + (int)(milisecond / MILISECONDS_PER_YEAR);
    }

    public int getDayOfYear() {
        long milisecondMinusYears = (milisecond % MILISECONDS_PER_YEAR);
        int dayOfYear = (int) ((milisecondMinusYears + MILISECONDS_PER_DAY)/ MILISECONDS_PER_DAY) - getTotalOfLeapYears();

        return dayOfYear;
    }

    public int getHour() {
        long milisecondMinusYears = (milisecond % MILISECONDS_PER_YEAR);
        long milisecondsMinusDays = (milisecondMinusYears % MILISECONDS_PER_DAY);   

        return (int) ((milisecondsMinusDays + MILISECONDS_PER_HOUR) / MILISECONDS_PER_HOUR);
    }

    public int getMinute() {
        long milisecondMinusYears = (milisecond % MILISECONDS_PER_YEAR);
        long milisecondsMinusDays = (milisecondMinusYears % MILISECONDS_PER_DAY);
        long milisecondsMinusHours = (milisecondsMinusDays % MILISECONDS_PER_HOUR);

        return (int) (milisecondsMinusHours / MILISECONDS_PER_MINUTE);
    }

    public int getSecond() {
        long milisecondMinusYears = (milisecond % MILISECONDS_PER_YEAR);
        long milisecondsMinusDays = (milisecondMinusYears % MILISECONDS_PER_DAY);
        long milisecondsMinusHours = (milisecondsMinusDays % MILISECONDS_PER_HOUR);
        long milisecondsMinusMinutes = (milisecondsMinusHours % MILISECONDS_PER_MINUTE);

        return (int) (milisecondsMinusMinutes / MILISECONDS_PER_SECONDS);
    }

    private int getTotalOfLeapYears() {
        int currentYear = getYear();
        int totalLeapYears = 0;
        for(int year = FIRST_YEAR; year <= currentYear; year++) {
            if(isLeapYear(year)) {
                totalLeapYears += 1;
            }
        }
        return totalLeapYears;
    }

    private int getMonth() {
        int dayOfYear = getDayOfYear();
        int month = 0;
        int leapYear = 0;

        if(isLeapYear(getYear())) {
            leapYear = 1 ;
        }

        if(dayOfYear < DAY_OF_YEAR_IN_END_OF_JANUARY) {
            month = 1;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_FEBRUARY + leapYear)) {
            month = 2;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_MARCH + leapYear)) {
            month = 3;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_APRIL + leapYear)) {
            month = 4;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_MAY + leapYear)) {
            month = 5;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_JUNE + leapYear)) {
            month = 6;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_JULY + leapYear)) {
            month = 7;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_AUGUST + leapYear)) {
            month = 8;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_SEPTEMBER + leapYear)) {
            month = 9;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_OCTOBER + leapYear)) {
            month = 10;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_NOVEMBER + leapYear)) {
            month = 11;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_DECEMBER + leapYear)) {
            month = 12;
        }else {
            throw new Error("An error occurred trying to get the day of the month");
        }

        return month;
    }

    private int getDayOfMonth() {
        int dayOfYear = getDayOfYear();
        int month = getMonth();
        int leapYear = 0;

        if(isLeapYear(getYear())) {
            leapYear = 1 ;
        }

        switch (month) {
            case 1:
                break;
            case 2:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_JANUARY; 
                break;
            case 3:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_FEBRUARY - leapYear; 
                break;
            case 4:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_MARCH - leapYear; 
                break;
            case 5:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_APRIL - leapYear; 
                break;
            case 6:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_MAY - leapYear; 
                break;
            case 7:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_JUNE - leapYear; 
                break;
            case 8:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_JULY - leapYear; 
                break;
            case 9:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_AUGUST - leapYear; 
                break;
            case 10:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_SEPTEMBER - leapYear; 
                break;
            case 11:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_OCTOBER - leapYear; 
                break;
            case 12:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_NOVEMBER - leapYear; 
                break;
            default:
                throw new Error("An error occurred trying to get the day of the month");
        }

        return dayOfYear;
    }

    private boolean isLeapYear(int year) {
        if ((year % 4 == 0) && ((year % 100 != 0) || (year % 400 == 0))) {
            return true;
        }else {
            return false;
        }
    }

    public String toString() {
        String date = "";

        date += getYear() + "-";
        date += getMonth() + "-";
        date += getDayOfMonth() + " ";

        date += getHour() + ":";
        date += getMinute() + ":";
        date += getSecond();

        return date;
    }

}

Any suggestion about the code, any idea that why I did

Any idea why I need to add another hour and a day?

Answer 1

public class MilisecondToDate {

Millisecond has two 'l's in it.

public class MillisecondToDate {

You were consistent in your spelling, so this typo appears a lot. I won't try to correct every instance but will fix it without comment where I happen to be editing code.

    public final int DAY_OF_YEAR_IN_END_OF_JANUARY = 31;
    public final int DAY_OF_YEAR_IN_END_OF_FEBRUARY = 59;
    public final int DAY_OF_YEAR_IN_END_OF_MARCH = 90;
    public final int DAY_OF_YEAR_IN_END_OF_APRIL = 120;
    public final int DAY_OF_YEAR_IN_END_OF_MAY = 151;
    public final int DAY_OF_YEAR_IN_END_OF_JUNE = 181;
    public final int DAY_OF_YEAR_IN_END_OF_JULY = 212;
    public final int DAY_OF_YEAR_IN_END_OF_AUGUST = 243;
    public final int DAY_OF_YEAR_IN_END_OF_SEPTEMBER = 273;
    public final int DAY_OF_YEAR_IN_END_OF_OCTOBER = 304;
    public final int DAY_OF_YEAR_IN_END_OF_NOVEMBER = 334;
    public final int DAY_OF_YEAR_IN_END_OF_DECEMBER = 365;

Given how you use this, this would be better written

    public final int[] END_DAY_OF_MONTH = new int[] {
            0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365 };

Then

        int leapYear = 0;

        if(isLeapYear(getYear())) {
            leapYear = 1 ;
        }

        if(dayOfYear < DAY_OF_YEAR_IN_END_OF_JANUARY) {
            month = 1;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_FEBRUARY + leapYear)) {
            month = 2;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_MARCH + leapYear)) {
            month = 3;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_APRIL + leapYear)) {
            month = 4;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_MAY + leapYear)) {
            month = 5;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_JUNE + leapYear)) {
            month = 6;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_JULY + leapYear)) {
            month = 7;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_AUGUST + leapYear)) {
            month = 8;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_SEPTEMBER + leapYear)) {
            month = 9;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_OCTOBER + leapYear)) {
            month = 10;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_NOVEMBER + leapYear)) {
            month = 11;
        }else if(dayOfYear < (DAY_OF_YEAR_IN_END_OF_DECEMBER + leapYear)) {
            month = 12;
        }else {
            throw new Error("An error occurred trying to get the day of the month");
        }

becomes

        if (dayOfYear <= END_DAY_OF_MONTH[0]) {
            throw new IllegalArgumentException("Cannot have non-positive day of year.");
        }

        if (dayOfYear <= END_DAY_OF_MONTH[1]) {
            return 1;
        }

        if (isLeapYear(calculateYear())) {
            dayOfYear--;
        }

        for (int month = 2; month < END_DAY_OF_MONTH.length; month++) {
            if (dayOfYear <= END_DAY_OF_MONTH[month]) {
                return month;
            }
        }

        throw new IllegalArgumentException(
                "Cannot have day of year more than number of days in year.");

And

        int dayOfYear = getDayOfYear();
        int month = getMonth();
        int leapYear = 0;

        if(isLeapYear(getYear())) {
            leapYear = 1 ;
        }

        switch (month) {
            case 1:
                break;
            case 2:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_JANUARY; 
                break;
            case 3:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_FEBRUARY - leapYear; 
                break;
            case 4:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_MARCH - leapYear; 
                break;
            case 5:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_APRIL - leapYear; 
                break;
            case 6:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_MAY - leapYear; 
                break;
            case 7:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_JUNE - leapYear; 
                break;
            case 8:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_JULY - leapYear; 
                break;
            case 9:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_AUGUST - leapYear; 
                break;
            case 10:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_SEPTEMBER - leapYear; 
                break;
            case 11:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_OCTOBER - leapYear; 
                break;
            case 12:
                dayOfYear -= DAY_OF_YEAR_IN_END_OF_NOVEMBER - leapYear; 
                break;
            default:
                throw new Error("An error occurred trying to get the day of the month");
        }

        return dayOfYear;

becomes

        int day = calculateDayOfYear();
        int monthNumber = calculateMonth();

        if (monthNumber > Month.FEBRUARY.getNumber() && isLeapYear(calculateYear()) {
            // leap years have an extra day in February, so subtract that out
            // for days after February
            day--;
        }

        return day - END_DAY_OF_MONTH[monthNumber - 1];

I'm not sure about throwing an IllegalArgumentException. There may be a better one. But I am sure that you should not just throw Error. The second method will throw an ArrayIndexOutOfBounds exception. Perhaps the first one should too.

It's also worth noting that these should never be called, as there should be no way to generate them. There's an argument that they should be left out. Because if you can't write a unit test for it, there's no point in having the code for it.

This requires an enum named Month with the month names as values. It should have a getNumber method that returns the one-indexed month of the year. In particular, it should return a 2 for Month.FEBRUARY.

I changed get to calculate wherever you were not accessing a field.

I disagree with this approach. Note that you have to recalculate each value every time you need it. I would rather calculate all the values once, save them, and just fetch them as needed. To do this, I would create a DateTime class with getters for each field (millisecond, second, minute, hour, day of year, day of month, month, year, etc.). Then your MillisecondToDate class could have have a convert method that would return a DateTime and take an epochMillisecond parameter. I suspect that you would find synergies in the conversion.

    private int getTotalOfLeapYears() {
        int currentYear = getYear();
        int totalLeapYears = 0;
        for(int year = FIRST_YEAR; year <= currentYear; year++) {
            if(isLeapYear(year)) {
                totalLeapYears += 1;
            }
        }
        return totalLeapYears;
    }

First, I would call this countLeapYears.

Second, why increment by one? Find the first leap year after FIRST_YEAR (1972) and increment by four.

Third, you shouldn't need to count. You can calculate this.

    public final int LEAP_CENTURY_YEAR_COUNT = 400;
    public final int EPOCH_LEAP_CENTURY_COUNT = FIRST_YEAR / LEAP_CENTURY_YEAR_COUNT;
    public final int LEAP_CENTURY_LEAP_YEAR_COUNT = 97;

    private calculateLeapYearsUntil() {
        int currentYear = calculateYear();
        if (currentYear < FIRST_YEAR) {
            return 0;
        }

        int sinceLeapCenturyCount = currentYear % LEAP_CENTURY_YEAR_COUNT;
        int leapCenturyCount = currentYear / LEAP_CENTURY_YEAR_COUNT - EPOCH_LEAP_CENTURY_COUNT;

        int leapYearCount = leapCenturyCount * LEAP_CENTURY_YEAR_COUNT;
        leapYearCount += sinceLeapCenturyCount / 4;
        leapYearCount -= sinceLeapCenturyCount / 100;

        return leapYearCount;
    }

I haven't tested this, so be careful of off-by-one errors and such. But an approach like this should calculate the number of leap years directly. It won't make much difference if the year is 2019, but if the year is 12019, then this should do significantly fewer calculations.

This solution won't work with years before 1970, but neither did your solution. I did my best to make the return values match yours.

        if ((year % 4 == 0) && ((year % 100 != 0) || (year % 400 == 0))) {
            return true;
        }else {
            return false;
        }

This pattern can be written more briefly and simply as

        return (year % 4 == 0) && ((year % 100 != 0) || (year % 400 == 0));

    public String toString() {
        String date = "";

        date += getYear() + "-";
        date += getMonth() + "-";
        date += getDayOfMonth() + " ";

        date += getHour() + ":";
        date += getMinute() + ":";
        date += getSecond();

        return date;
    }

You can use either String.format or StringBuilder here.

    public String toString() {
        StringBuilder result = new StringBuilder();

        result.append(calculateYear()).append('-')
              .append(calculateMonth()).append('-')
              .append(calculateDayOfMonth()).append(' ');

        result.append(calculateHour()).append(':')
              .append(calculateMinute()).append(':')
              .append(calculateSecond());

        return result.toString();
    }

The best thing that I can say about using += with strings is that it usually uses StringBuilder behind the scenes. The problem here is that it might use more than one StringBuilder.

Answer 2

Bug - doesn't handle leap years

I got as far as your first function:

public int getYear() {
    return FIRST_YEAR + (int)(milisecond / MILISECONDS_PER_YEAR);
}

and immediately saw that this wasn't going to work, because the number of milliseconds in a year isn't constant. In leap years, there is an extra day's worth of milliseconds. So for example, if your input value were Dec 31, 1972, your getYear() function would mistakenly return 1973. It gets worse the farther you get from 1970.

Bug - can return negative day of year

Moving on to the second function:

public int getDayOfYear() {
    long milisecondMinusYears = (milisecond % MILISECONDS_PER_YEAR);
    int dayOfYear = (int) ((milisecondMinusYears + MILISECONDS_PER_DAY)/ MILISECONDS_PER_DAY) - getTotalOfLeapYears();

    return dayOfYear;
}

Here, there is some attempt to adjust for leap years, but it isn't correct. The first part of the calculation for dayOfYear produces a number in the range 1..365, but then you subtract the number of leap years, which could be some large number. Suppose the input year is 2010 and there were 10 leap years since 1970. Your function will return a number in the range -9..355, which is definitely wrong.

This function should return a value in the range 1..365 for regular years or 1..366 for leap years.

A strange question

I find it odd that you asked this question:

Any idea why I need to add another hour and a day?

If you added that code, I assume you must have had a reason to do it, so wouldn't you already know why? What happens if you don't add the extra hour and day? Did you write this code?

In response to comment

OK so you tested the current time 3/27/19 10:14 and your program gave you 3/26/19 9:14, so you added a day and an hour to fix the problem. However, you made the fix without actually figuring out why your logic was wrong in the first place, so your fix may not be correct. When bugs occur, you should determine the cause of the problem so that you can apply the correct fix, otherwise you won't know whether you have really fixed the problem.

One way to do this is to test simple known values and examine the results. For example, if you give the input 0 to the program, it should refer to 1/1/1970 00:00 and your program should return 1 for the day of the year and 0 for the hour. Prior to your change, your program would give 0 for the day of the year and 0 for the hour. After your change, your program correctly gives 1 for the day of the year but incorrectly 1 for the hour.

For the day of year, the reason for the bug is simple. Day of year is supposed to be in the range 1..365 (i.e. one-based, ignoring leap year for now) but your division results in a number in the range 0..364 (i.e. zero-based). Therefore you need to add one to shift from zero-based to one-based. However, note that once you change your result to be one-based, all functions that internally use dayOfYear() need to adjust for that, namely getMonth().

For the hour, the reason for the problem is not quite as clear. Assuming you want to return a military time hour (0..23), then the result you calculated is already in the correct range, and in fact, the correct value. I suspect that the reason your hour is one hour off from "now" is that you are on daylight savings time. Again, if you had tested various known times and edge cases, such as 12/31/1970 23:59, you would have seen that your current program returned 24 for the hour, which is not correct.