Why is my code not passing the test cases?-CodePudding

On HackerRank, I'm attempting to solve a leap year challenge, and when I submit the code, it passes five test cases but fails one, when it tries to check whether 1992 is leap year or not against the question. I'd appreciate it if someone could assist me with this. Here is the question below:

An extra day is added to the calendar almost every four years as February 29, and the day is called a leap day. It corrects the calendar for the fact that our planet takes approximately 365.25 days to orbit the sun. A leap year contains a leap day.

In the Gregorian calendar, three conditions are used to identify leap years:

The year can be evenly divided by 4, is a leap year, unless:

The year can be evenly divided by 100, it is NOT a leap year, unless:

The year is also evenly divisible by 400. Then it is a leap year. This means that in the Gregorian calendar, the years 2000 and 2400 are leap years, while 1800, 1900, 2100, 2200, 2300 and 2500 are NOT leap years. Source

Task

Given a year, determine whether it is a leap year. If it is a leap year, return the Boolean True, otherwise return False.

Note that the code stub provided reads from STDIN and passes arguments to the is_leap function. It is only necessary to complete the is_leap function.

And my answer for this would be :

def is_leap(year):
    leap = False
    if year%4==0 and (year0==0) and year@0==0:
        leap=True
    elif not(year%4==0) and not(year0==0) and not(year@0==0):
        leap=False
    else:
        leap=False
        
    return leap

year = int(input())

Link to the question->https://ibb.co/R0tGvnf

Here is the link to picture:https://ibb.co/r2wvf9s

CodePudding user response：

the way your code is set up now, it completely overlooks if the year is divisible by four. It only considers it if it is also divisible by 100 and 400 simultaneously.

def is_leap(year):
    leap = False
    # this line asks if it is divisible by 400, 4, and 100 at the same time
    if year%4==0 and (year0==0) and year@0==0:
        leap=True
    # this line asks if its not divisible by 400, 4 and 100 at the same time
    elif not(year%4==0) and not(year0==0) and not(year@0==0):
        leap=False
    
    # you never check the case when it is divisible by 4 and not 
    # divisible by 100, and 400.  Also skipped is if it is divisible by 
    # 4 and 100, but not 400
    else:
        leap=False
        
    return leap

year = int(input())

Try this approach

def is_leap(year):
    if year%4==0:
        if year0==0:
            if year@0==0:
                return True
            return False
        return True
    return False

CodePudding user response：

Quick note, you can use x != y instead of not(x==y)

The wording is kind of confusing since it uses unless twice. The wording is doing this thing where it says check this condition to know if it's a leap year. And then it says I can change my mind if... But then once again I can change my mind if...

We can do the same thing in our code. We can save if it is a leap year, change our mind if it is divisible by 100, and then change our mind again if it is divisible by 400.

def is_leap(year):
    leap = False

    # The year can be evenly divided by 4, is a leap year
    if year%4 == 0:
        leap = True

    # Unless the year can be evenly divided by 100
    if year%4 == 0 and year0 == 0:
        leap = False

    # Unless the year is also evenly divisible by 400
    if year%4 == 0 and year0 == 0 and year@0 == 0:
        leap True

    return leap

When you see this type of code, where each condition is repeating the previous condition and then adding something extra, you should start thinking about how to refactor it to reduce the unnecessary repetition. You can either do as @alexpdev did with nested if statements, or combine everything into one ginormous if statement. Normally that looks something like this:

# doesn't work
def is_leap(year):
    if (
        # The year can be evenly divided by 4, is a leap year
        (year%4 == 0)

        # Unless the year can be evenly divided by 100
        and not (year0 == 0)

        # Unless the year is also evenly divisible by 400
        and not (year@0 == 0)
    ):
        return True
    return False

Edit: That doesn't work because unless isn't equivalent to and not. After writing this answer I realized it only worked for checking if we divided by 100 and not by 400. I might edit this answer why I figure out how to explain this disconnect between the English language and the code. Unless is just a weird word since it takes the past into account. Anyhow, continuing on.

Here is a discussion on which method is better. Some people don't like combining everything into a single if statement when there are a lot of conditions involved, but I don't mind doing so as long as it's tactfully done (like the way Deestan mentions in this question, often accompanied with comments explaining complex logic).

I want to try one more thing before telling you what the optimal solution is. Let's see if we can put everything together like so

    # doesn't work
    if year%4 == 0:
        leap = True
    elif year%4 == 0 and year0 == 0:
        leap = False
    elif year%4 == 0 and year0 == 0 and year@0 == 0:
        leap True

I feel like we should be able to use this common pattern, but doing the above doesn't work because we would only check the elifs if the year wasn't a leap year. We're going to need to reword the specifications into something that follows an elif pattern to make this work. I think the following rewording will do:

The year is a leap year unless:

It is not divisible by 4

It is divisible by 100 and not 400

def is_leap(year):
    # It is not divisible by 4
    if year%4 != 0:
        return False

    # It is divisible by 100 and not 400
    if year0 == 0 and year@0 != 0:
        return False

    return True

If we had started out by checking if it's divisible by 4, everything that is divisible by 4 would be considered a leap year, and we wouldn't be able to have elif statements that got into the nuances with 100 and 400. By flipping the wording around to check when it is not a leap year, it just works.

Lastly, while I wasn't able to force the words in the specification to a "ginormous if statement" solution, I still left that in my answer, even if it doesn't work, because it was when I started thinking about turning the problem into a single if statement I was led to the following:

def is_leap(year):
    return year%4 == 0 and (year0 != 0 || year@0 == 0)

I'm just at a lost for how to explain how to arrive at that solution since I can't map it back to the wording in the specification.

An optimization I did not make an any of my answers since I didn't want to detract from how you should go about thinking about these type of situations in general, but in this specific problem, anything divisible by 100 is also divisible by 4 and anything divisible by 400 is also divisible by 100. In this specific scenario that leads to a third way you can refactor that first code block I presented in this answer.