Java Code for calculating Leap Year, is this code correct ?

Question

Hello

I am following "The Art and Science of Java" book and it shows how to calculate a leap year. The book uses ACM Java Task Force's library.

Here is the code the books uses:

import acm.program.*;

public class LeapYear extends ConsoleProgram {
    public void run()
    {

        println("This program calculates leap year.");
        int year = readInt("Enter the year: ");     

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

        if (isLeapYear)
        {
            println(year + " is a leap year.");
        } else
            println(year + " is not a leap year.");
    }

}

Now, this is how I calculated the leap year.

import acm.program.*;

public class LeapYear extends ConsoleProgram {
    public void run()
    {

        println("This program calculates leap year.");
        int year = readInt("Enter the year: ");

        if ((year % 4 == 0) && year % 100 != 0)
        {
            println(year + " is a leap year.");
        }
        else if ((year % 4 == 0) && (year % 100 == 0) && (year % 400 == 0))
        {
            println(year + " is a leap year.");
        }
        else
        {
            println(year + " is not a leap year.");
        }
    }
}

Is there anything wrong with my code or should i use the one provided by the book ?

EDIT :: Both of the above code works fine, What i want to ask is which code is the best way to calculate the leap year.

Answer 1

Why don't you try to input 2004, 2000 and 2100 to test whether the output will be the same?

Answer 2

The pseudo code from Wikipedia looks much simpler:

if year modulo 400 is 0 then leap
 else if year modulo 100 is 0 then no_leap
 else if year modulo 4 is 0 then leap
 else no_leap

Answer 3

They look the same to me, though note that this line in your code has some redundancy:

else if ((year % 4 == 0) && (year % 100 == 0) && (year % 400 == 0))

could be replaced by:

else if (year % 400 == 0)

If a number is a multiple of 400 then it's automatically also a multiple of 100 and 4.

Answer 4

I suggest you put this code into a method and create a unit test.

public static boolean isLeapYear(int year) {
    assert year >= 1583; // not valid before this date.
    return ((year % 4 == 0) && (year % 100 != 0)) || (year % 400 == 0);
}

In the unit test

assertTrue(isLeapYear(2000));
assertTrue(isLeapYear(1904));
assertFalse(isLeapYear(1900));
assertFalse(isLeapYear(1901));

Answer 5

It's almost always wrong to have repetition in software. In any engineering discipline, form should follow function, and you have three branches for something which has two possible paths - it's either a leap year or not.

The mechanism which has the test on one line doesn't have that issue, but generally it would be better to separate the test into a function which takes an int representing a year and returns a boolean representing whether or not the year is a leap year. That way you can do something with it other that print to standard output on the console, and can more easily test it.

In code which is known to exceed its performance budget, it's usual to arrange the tests so that they are not redundant and perform the tests in an order which returns early. The wikipedia example does this - for most years you have to calculate modulo 400,100 and 4, but for a few you only need modulo 400 or 400 and 100. This is a small optimisation in terms of performance ( at best, only one in a hundred inputs are effected ), but it also means the code has less repetition, and there's less for the programmer to type in.

Answer 6

The correct implementation is:

public static boolean isLeapYear(int year) {
  Calendar cal = Calendar.getInstance();
  cal.set(Calendar.YEAR, year);
  return cal.getActualMaximum(DAY_OF_YEAR) > 365;
}

But if you are going to reinvent this wheel then:

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

Answer 7

Pseudo code from Wikipedia translated into the most compact Javaese...

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