Leap year program prints wrong output

Hi,

This program is supposed to print all the leap year between any two given years. It seems to work when the gap between years is huge, but when I enter 1991 and 2005 for example, it will only tell me that 2000 is a leap year, when years such as 92,96, and 04 are also leap years, right?

Any ideas where Iv'e gone wrong?

Code:

---


#include <stdio.h>
#include <process.h>
#include <values.h>

int main(void)
{
  char  your_name[12];
  int  year_start;
  int  year_finish;
  int  leap_yr_input = 0;
  int  leap_yr_test = 0;
  int  maxint_value;
  int  i;
  int  yr_test;
  int  nearest_leap_yr;

  system("cls");

  printf("\nPlease enter your name(e.g. Jan): ");
  scanf("%s", your_name);

  maxint_value = MAXINT;

  do
  {
      printf("\nEnter year to begin with between 1 and %d: ", maxint_value - 1);
      scanf("%d", &year_start);

      printf("\nEnter an ending year between %d and %d(e.g. %d): ",
            year_start + 1, maxint_value, year_start + 1);
      scanf("%d", &year_finish);

      printf("\n");

      if(year_start >= year_finish)
      {
        printf("\nInvalid input!");
        printf("\nThe ending year must be bigger than the starting year!");
        printf("\n\nRe-enter years");
      }
  }
  while( year_start >= year_finish );

  printf("\nHello %s!", your_name);
  printf("\n");
  printf("\nYour starting year is: %d", year_start);
  printf("\nYour ending year is: %d", year_finish);
  printf("\n\n");

  for( i = year_start; i <= year_finish; i++ )
  {
      if( i % 4000 != 0 )
      {
        if( i % 4 == 0 )
        {
            if( i % 400 == 0 )
            {
              leap_yr_input = 1;
              printf("\nThe year %d is a leap year", i);
            }
        }
      }
  }

  if( leap_yr_input == 1 )
      printf("\n\nThis is the end of LEAP year list");

  if( leap_yr_input == 0 )
  {
      printf("\nThere are no leap years in your input range");

      for( yr_test = year_finish + 1; yr_test <= maxint_value; yr_test ++)
      {
        if(yr_test % 4000!= 0)
        {
            if(yr_test % 4 == 0)
            {
              if(yr_test % 400 == 0)
              {
                  leap_yr_test = 1;
                  nearest_leap_yr = yr_test;
                  break;
              }
              else if(yr_test % 100 != 0)
              {
                  leap_yr_test = 1;
                  nearest_leap_yr = yr_test;
                  break;
              }
            }
        }
      }

      if( leap_yr_test == 0)
      {
        printf("\nThere is NO leap year in the range between ");
        printf("your input and MAXINT");
      }

      if(leap_yr_test == 1)
      {
        printf("\n\nThe nearest leap year that is greater than your input ");
        printf("is: %d", nearest_leap_yr);
      }
  }

  printf("\n");
  printf("\nThanks for using this program! ");
  printf("Good-bye!");

  system("pause");
}

---

first things first

Code:

---

void main()

---

Take out the i % 400 line in your first loop. Otherwise you'll only get years divisible by 400.

Theres a problem with how the leap year is checked.

This will return 1 if the year is a leap year.

Code:

---

int LeapYear(int iYear)
{
        int iReturn=0;

        if ( !(iYear % 4) )
        {
                iReturn=1;
                if ( (iYear % 100) && !(iYear % 400) )
                        iReturn=0;
        }

        return iReturn;
}

---

Hope this helps
-Joe

This will probably execute a little faster:

Code:

---

#define IsLeapYear(y) (!((y)%4) && ((y)%100 || !((y)%400)))

---

at the cost of confusing code
'macro functions are the tool of the devil!'

Quote:

---

Originally Posted by sand_man

at the cost of confusing code
'macro functions are the tool of the devil!'

---

Huh? Maybe it's a personal preference issue. Even libc uses macros extensively. The only way it would act any different is if you passed something that had issues with being evaluated more than once to the macro (i.e. IsLeapYear(year++); or IsLeapYear(rand()%4000);)

But there's no need for pointless macro-isation of everything simply on the basis of "because I can"

Just because your expression doesn't contain any if statements doesn't mean it isn't riddled with hidden jump instructions for example.

I didn't macri-ize it just because I could. I found KonArtis' function unnecessarily difficult to follow so I originally rewrote it as:

Code:

---

int IsLeapYear(int y)
{
  return (!(y%4) && (y%100 || !(y%400)));
}

---

But since it turned out to be a simple 1-liner I thought it might do well as a macro. It's not like I try to turn every function I see into a macro "just because I can".

And when I said the macro will run faster I based it on 2 assumptions: 1) no stack overhead from calling the function, 2) eliminating the local variable iReturn in KonArtis' function.

Still, I'm 99.9% sure the macro will run faster than KonArtis' function on most computers. If you prefer the function then go for it.