1

I wrote this code to determine whether or not an inputted year is a leap year. Meaning, those divisible by 4 and 400 are leap years, and by 100 or something else are not.

But, my program always returns true for the boolean value, so that output will believe every year is a leap year.

Here's my code so far: 

#include <iostream>
#include <cmath>
#include <string>
#include <iomanip>

using namespace std;
 bool leap_year(int year);
int main()
{
    int year; 
     bool leap_year(int year);
    cout << "Please input the year in question: ";
    cin >> year;
    if (leap_year == false)
    {
        cout << "The year is not a leap year.  ";
    }
    else
    {
        cout << "The year is a leap year. ";
    }
    return 0;
}
  bool leap_year(int year)
{

    if (year % 4 == 0)
    {
        bool leap_year = true;
    }
    else if (year % 400 == 0)
    {
        bool leap_year = true;
    }
    else if (year % 100 == 0)
    {
        bool leap_year = false;
    }
    else
    {
        bool leap_year = false;
    }

    if (bool leap_year = false)
    {
        return false;
    }
    else
    {
        return true;
    }
}
Yu Hao
  • 119,891
  • 44
  • 235
  • 294
marihikari
  • 25
  • 1
  • 5
  • 1
    It shouldn't always return `true` because [it shouldn't compile](http://coliru.stacked-crooked.com/a/71aea1ab488cdd36). – chris Nov 02 '15 at 02:40
  • It definitely compiles. I am using Visual Studio 2015 Express edition as a compiler. – marihikari Nov 02 '15 at 02:48
  • Funny enough, GCC also compiles it (with a warning). However, from some reading, I still believe Clang is right not to. Edit: GCC is wrong, and I'm thinking it's because it still accepts `false` as a null pointer constant (i.e., it lets you do `int* p = false;`). – chris Nov 02 '15 at 03:53

4 Answers4

8

You are declaring a bunch of local variables

else if (year % 100 == 0)
{
    bool leap_year = false;
}

As soon as the } parentheses, this variable goes out of scope and so does the value you have stored.

if (bool leap_year = false)
{
    return false;
}

This is defining a variable leap_year and assigning it false. The if in turn will evaluate false, hence it will always go to else condition.

I have taken the liberty to rewrite a portion of the program. You can now see how the call is being made to the function. Also in the function a local variable is_leap_year is used to store the return value, and finally being returned. I have also corrected the logic, as earlier the first %4 check would be true and none of the other if statements would be executed, which is not what you want.

#include <iostream>
#include <cmath>
#include <string>
#include <iomanip>

using namespace std;
bool leap_year(int year);
int main()
{
    int year; 
    cout << "Please input the year in question: ";
    cin >> year;
    if (leap_year(year) == false)  //Call the function and check if return is false
    {
        cout << "The year is not a leap year.  ";
    }
    else
    {
        cout << "The year is a leap year. ";
    }
    return 0;
}
  bool leap_year(int year)
{
    bool is_leap_year = false;
    if (year % 4 == 0)
    {
       is_leap_year = true;
    }
    if (year % 100 == 0)
    {
        is_leap_year = false;
    }
    if (year % 400 == 0)
    {
        is_leap_year = true;
    }
    return is_leap_year;
}
Imran
  • 413
  • 5
  • 14
1

It should be IF ((year%4==0 && year%100 !=0) || (year%400 == 0) ) return ISLEAPYEAR.

Xubin Zhuge
  • 21
  • 1
1

Your problem is that your condition whether to return true or false is:

if (bool leap_year = false)
{
    return false;
}
else
{
    return true;
}

You are assigning the value false to the variable leap_year which you initialize in the condition statement. The assignment operator is defined: T& T::operator =(const T2& b) which means that you are simply always evaluating false in your condition.

One way to solve this would be to declare leap_year at the top of bool leap_year(int year) rather than every time you use it (which is pointless behavior.) So your function should look like this:

bool leap_year(int year) {
    bool leap_year = false;

    if (year % 4 == 0) {
        leap_year = true;
    } else if (year % 400 == 0) {
        leap_year = true;
    } else if (year % 100 == 0) {
        leap_year = false;
    }

    if (leap_year == false) {
        return false;
    } else {
        return true;
    }
}

But a better solution would be to use the functionality that C++ already provides:

bool leap_year(int year) {
    tm bar = { 0, 0, 0, 29, 1, year - 1900 };

    return static_cast<time_t>(-1) != mktime(&bar) && bar.tm_mday == 29 && bar.tm_mon == 1 && bar.tm_year == year - 1900;
}

[Live Example]

Community
  • 1
  • 1
Jonathan Mee
  • 37,899
  • 23
  • 129
  • 288
  • That would be a good solution. Somehow when I try something similar it still won't return a false value. Will keep working, but thanks for your response :) – marihikari Nov 02 '15 at 04:45
  • @marihikari +1 It appears you're correct. There seems to be a bug in gcc 5.1.0 and Visual Studio 2015. I've opened a new question on what's going on there: http://stackoverflow.com/questions/33477379/mktime-only-handling-leap-years-on-clang – Jonathan Mee Nov 02 '15 at 12:21
  • @marihikari Success [this answer](http://stackoverflow.com/a/33478052/2642059) contains a fully cross platform standard implemented solution! I have updated my answer to contain it. – Jonathan Mee Nov 02 '15 at 12:55
1
//You can simplify it like this:
bool leap_year(int year)
{
    bool leap_year  = false;
    if ((year % 4 == 0 && year%100 !=0) || year % 400 == 0)
    {
        bool leap_year = true;
    }
    return leap_year; 
}

To avoid this:

if (leap_year = false)
    {
        return false;
    }

You can check boolean values like this

if (leap_year)
 {
        // code to execute when leap_year is true
 }
Ankit Deshpande
  • 3,476
  • 1
  • 29
  • 42