CODEWITHSUNDEEP
c++dateif-statementstatements
Sehajpreet Singh
Sehajpreet Singh

Reputation: 9

Leap year program doesn't seem to run

Question - A year with 366 days is called a leap year. A year is a leap year if it is divisible by four (for example, 1980), except that it is not a leap year if it is divisible by 100 (for example, 1900); however, it is a leap year if it is divisible by 400 (for example, 2000). There were no exceptions before the introduction of the Gregorian calendar on October 15, 1582. Write a program that asks the user for a year and computes whether that year is a leap year.

This is what I have so far, and the program doesn't seem to run for years greater than 1582. Could someone help me out why? Thanks a bunch

using namespace std;

int main()
{
cout<< "Pleas enter a year: " <<endl;
int year = 0;
cin >> year;

if (year <= 1581)
{
    if (year % 4 == 0)
    {
        cout << "It's a leap year, wow! " << endl;
    }

    else 
    {
        cout << "It's not a leap year " << endl;

    }
}

else if (year > 1581)
{
    if (year % 4 == 0)
    {
        if (year % 100 == 0)
        {
            cout<< "It is not a leap year " << endl;
        }
        else if (year % 400 == 0)
        {
            cout<< "It is a leap year, Wow!" << endl;
        }
    }

}

else
{
    cout<< "You entered a wrong year number "<< year<<  endl;
}

return 0;
}

Upvotes: 0

Views: 1998

Answers (7)

Davislor
Davislor

Reputation: 15144

It’s a good idea to simplify your conditionals. A general method for this is to convert to a normal form—either conjunctive or disjunctive—and put the tests that are most likely to short-circuit, first. For simple cases such as this, you can just eyeball it.

In this case, conjuctive normal form is extremely simple:

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

That is, the year is divisible by four and any of the three conditions of the Gregorian calendar reform do not hold. Since the first && term that is false, and the first || term that is true, short-circuit the expression, we want to put the clauses that are most likely to short-circuit, first.

Note that you can code-golf year%100 != 0 to year%100 inside a conditional expression, and year%2000 == 0 to !(year%2000), if you find that easier to read.

It makes sense to move this to a helper function. We can mark it constexpr to give the compiler a hint that it can calculate whether constants are leap years or not at compile-time.

I don't like to post complete answers to what look like homework problems, but that ship has sailed.

#include <cstdlib>
#include <iostream>

using std::cin;
using std::cout;

constexpr bool is_leap(const int year)
{
  // Conjunctive normal form:
  return ( year%4 == 0 &&
           ( year < 1582 || year%100 != 0 || year%400 == 0 ) );
}

int main()
{
  int year = 0; // Used only within the body of the loop that sets it.

  while (cin >> year)
    cout << year
         << ( is_leap(year) ? " is a leap year.\n"
                            : " is not a leap year.\n" );

  return EXIT_SUCCESS;
}

Even for a trivial program such as this, there’s a design decision worth thinking about: do we declare int year; uninitialized, or initialize it to a bad value such as int year = 0;? In this case, it’s safe either way, because year is only used inside the body of the loop that sets it. If we don’t initialize it, though, we might later refactor the code to use year outside the loop, and then there might be a code path where year is used before it’s initialized (causing undefined behavior!) On the other hand, if we do initialize year, we might prevent the compiler from noticing that there’s a path where it’s used before it was initialized for real. I personally prefer to initialize to an invalid value and assert that it has been updated before use.

Upvotes: 0

Abdisamad Khalif
Abdisamad Khalif

Reputation: 825

Or you can simply write the following program:

#include<iostream>
using namespace std;

int main()
{
    int year;
    cout << "Enter year: ";
    cin >> year;

    if ((year % 4 == 0 && year % 100 != 0) || year % 400 == 0){
        cout << "It is a leap year" << endl;
    }
    else{
        cout << "It is not a leap year" << endl;
    }
    return 0;
}

Upvotes: -1

user4924658
user4924658

Reputation:

Without check if year > 1582

#include<iostream>
using namespace std;

int main()
{
    int year;
    cout<< "Please enter a year: " <<endl;
    cin >> year;

    if( year%25 && !(year&3) || !(year&15) )
        cout << "It's a leap year!" << endl;
    else
        cout << "It's not a leap year!" << endl;

    return 0;
}

Upvotes: 1

max66
max66

Reputation: 66200

I suggest

if (    ((year % 400) == 0)
     || (   ((year % 4) == 0) // or (year & 0x3) == 0
         && (   (year <= 1581)
             || ((year % 100) != 0) )))
   cout << "It's a leap year, wow! " << endl;
else
   cout << "It's not a leap year " << endl;

or

if (    ((year % 4) == 0) // or  year & 0x3 == 0
     && (   (year <= 1581)
         || ((year % 100) != 0)
         || ((year % 400) == 0) ) )
   cout << "It's a leap year, wow! " << endl;
else
   cout << "It's not a leap year " << endl;

Upvotes: 0

user5794376
user5794376

Reputation:

You are making it very complex. I don't think you need to care about whether the year is greater than 1582 or not (for a 4 digit number) provided that a leap year is one which is:

• Divisible by 400
OR
• NOT divisible by 100 AND divisible by 4.
Using unnecessary nested ifs can make your code long and error prone. Try this method: 

#include<iostream.h>
int main(){
    int y=0;
    cout << "Enter a year\n";
    cin >> y;
    cout <<"\n"<<y;
    if(y%400==0 || (y%100!=0 && y%4==0))
    cout <<" is a leap year";
    else
    cout <<" is not a leap year";
    return 0;
}

Upvotes: 1

Viktor Mellgren
Viktor Mellgren

Reputation: 4506

You are missing 2 else statement

using namespace std;

int main()
{
cout<< "Pleas enter a year: " <<endl;
int year = 0;
cin >> year;

if (year <= 1581)
{
    if (year % 4 == 0)
    {
        cout << "It's a leap year, wow! " << endl;
    }

    else 
    {
        cout << "It's not a leap year " << endl;

    }
}

else if (year > 1581)
{
    if (year % 4 == 0)
    {
        if (year % 100 == 0)
        {
            cout<< "It is not a leap year " << endl;
        }
        else if (year % 400 == 0)
        {
            cout<< "It is a leap year, Wow!" << endl;
        }
        // <----------- Here you are missing an else
    }
    // <----------- Here you are missing an else
}

else
{
    cout<< "You entered a wrong year number "<< year<<  endl;
}

return 0;
}

Upvotes: 0

pmakholm
pmakholm

Reputation: 1548

You are missing a number of cases in the handling of years after 1581.

  • printing "Not leap year" unless (year % 4 == 0)
  • The third case where a year divisible by 4 is neither divisible by 100 nor 400

It as simple as you have just not written any code that is run when year is 2004, for example.

Upvotes: 0

Related Questions