views:

183

answers:

10
import java.util.Scanner;

public class GregorianYear

{
    private int year;

    public GregorianYear(int a)

    {
     year = a;
    }

    public void SetYear()
        {
     System.out.println( "The year is: " );
     Scanner kbd = new Scanner( System.in );
         year = kbd.nextInt();
    }

    public int getYear()
    {
     return year;
    }

    public boolean isLeapYear()
    {
     if ( year > 1852 )
     {
      if ( year % 100 == 0)
      {
       if ( year % 400 == 0)
       {
        return false;
       }
       else
       {
        return true;
       }
      }
     }
    //Compiler says I need a return statement here.
    }
}

I'm programming a (should be) simple program for class, and when I don't have anything there, it says I need a return statement. I assume I can only return a boolean statement, so I enter return isLeapYear();. When I do this, my test method (another file with a public static void main) runs and causes a StackOverflow error at the line where I entered the return statement. What am I doing wrong?

A: 

Of course you need a return statement after the outer if. Because the year can be <= 1852.

If your method should only handle years after 1852, you can throw an exception instead.

tangens
+2  A: 

Basically if your method declares a return value, so all your code paths in this method must return something, in the case of the code you posted, what if the year<=1852 ?? what should be the return value in this case??

bashmohandes
A: 

Hint: If you use return isLeapYear(); in the body of the isLeapYear() method you are telling it to call itself recursively.

Hint 2: Fix up the indentation of your code and it will be easier for you to understand what is wrong with it.

Stephen C
+7  A: 
Esko
A: 

When you write "return IsLeapYear()" you cause an infinte loop. Since you don't care about years before 1852, just return true or false and get A-...

Faruz
A: 

You have to add return false;. Because the method needs to return a value(boolean). And if your first condition in your if isn't true, everything will be skipped. Your first condition checks year<=1852. Now imagine: what will he return if your year is <= 1852.

Martijn Courteaux
+1  A: 

If you call isLeapYear again it will run forever. But use proper identation.

if ( year > 1852 )
{
   if ( year % 100 == 0)
   {
      if ( year % 400 == 0)
      {
         return false;
      }
      else
      {
         return true;
      }
   }
}

as you can see, you do not have a else statement for if (year > 1852) and if (year %100 == 0) therefore the compiler can't be sure that a value is returned.

lasseespeholt
+1  A: 

You need to return a value for every code path, including the one where the year predates 1853. In your code, that case isn't taken into account.

The following function will work as you wish. I'm assuming you actually mean 1582 since that was the year the Gregorian calendar started being used (in the Catholic world, anyway). Since it was enacted in October of that year, 1582 wasn't itself a leap year despite meeting the 4/100/400 rules.

I've reformatted the code to what I consider a more readable style - I'm not a big fan of returns followed by elses since they break up the meaning in my opinion (others may disagree but I believe this form makes indentation problems easier to spot). And, on top of that, you don't seem to take the every-four-year rule into account at all.

public boolean isLeapYear() {
    // No leap years before Greg the Pope.

    if ( year < 1583 )
        return false;

    // Multiples of 400 are leap years.

    if ( year % 400 == 0)
        return true;

    // Multiples of 100 (other than multiples of 400) are not.

    if ( year % 100 == 0)
        return false;

    // Multiples of 4 are, assuming they're not multiples of 100.

    if ( year % 4 == 0)
        return true;

    // All other aren't.

    return false;
}

Of course, the ideal solution is probably just to use GregorianCalendar.isLeapYear(int year), documented here. Very rarely do you have to write such a basic piece of code (other than for homework) since the Java class libraries provide a huge array of useful things.

The whole class, in all its beauty (including the ability to change the Julian-Gregorian cutover date) is documented here.

paxdiablo
A: 

Calling your own method without changing the arguments if a sure way to go into infinite recursion, calling your method again until the stack runs out of space.

You ask about return statements and that is where you can improve your code and fix the recursion.

As a rule try to have only one return statement. Doing that in this case would result in:

public boolean isLeapYear()
{
    boolean result = false;
    if ( year > 1852 )
    {
            if ( year % 100 == 0)
            {
                    if ( year % 400 != 0)
                    {
                            result = true;
                    }
            }
    }
    return result;
}

Btw I seem to recall that leap years have something to do with multiples of 4 too :-)

rsp
Less of a rule than a guideline to me. Especially if the multi-return version aids readability without sacrificing the ability to understand all the code paths - with a function this small, it should be easy (although admittedly, not so for larger functions).
paxdiablo
You're right, with rule I mean guideline for all situations that don't have a good reason to do otherwise. For functions like this 1 return is much more readable to me.
rsp
A: 

The leap year rule of the Julian calendar, which was used before the Gregorian calendar, is that every fourth year is a leap year. The Gregorian calendar keeps this rule and just adds exceptional rules for full centuries and multiples of 400 years.

Your code only computes the exceptional rules but misses the basic rule (apart from the missing return statements).

starblue