3
\$\begingroup\$

I have done the leap year check on https://exercism.io already in a lot of languages. Today I came back to the exercise in Java and was playing around with some maybe more funny ways to do the check. The following more functional one kept me thinking of:

import java.util.stream.IntStream;

class Leap {

    boolean isLeapYear(final int year) {
        return IntStream.of(4, 100, 400)
            .filter(divisor -> year % divisor == 0)
            .count() % 2 == 1;
    }

}

What do you think? Is this readable? At least it seems to be extensible in case more rules will ever get added ;)

Another funny fact: due to prime factorization we actually could also use the IntStream.of(2, 25, 16) (but for sure that doesn't help in readability).

\$\endgroup\$
4
  • \$\begingroup\$ please don't keep updating you code :) \$\endgroup\$ Apr 24, 2020 at 10:10
  • 2
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Vogel612
    Apr 24, 2020 at 10:15
  • 1
    \$\begingroup\$ This isn’t more functional-style than other pure approaches like return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0);, note. \$\endgroup\$
    – Ry-
    Apr 24, 2020 at 10:22
  • \$\begingroup\$ @Ry I didn't and wouldn't claim that return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0); isn't a functional implementation. It has no side effects and the result only depends on the input. \$\endgroup\$ Apr 24, 2020 at 10:23

2 Answers 2

4
\$\begingroup\$

What do you think?

It certainly is clever. That is not always a good thing, because it might be harder to understand what is going on (you have to be at least as clever)

Is this readable?

Not really. I can't get from a glance that 100 is treated differently from 400 and why. Also there is no room for comments close to you code.

If you make a set of ifs you could do something like:

if (year % 400 ... ) // if 400 then a leap year
{..
..}

In your IntStream you could do something like:

   IntStream.of(  4,    //yes
                100,    //no
                400     //yes
               )

But then you kinda miss the 'one-liner' point of your solution.

Is it efficient?

Not per se. Most efficient would be to first check divisibility by 400, if true, return true, etc.

Prime factorization funny fact

This in incorrect; because it would label 1998 as leap year. Or do you mean something different?

\$\endgroup\$
3
  • \$\begingroup\$ Funny fact: should be fixed by the change to .takeWhile(). I posted the wrong version of the code. \$\endgroup\$ Apr 24, 2020 at 9:52
  • \$\begingroup\$ Efficiency: (with the .takeWhile() now) I also have an “early return” and I will do it more often. For 3/4 of the years it will return after doing only a single test. When testing for 400 first, it's only 1/400 of the cases that return after the first check. \$\endgroup\$ Apr 24, 2020 at 9:57
  • 1
    \$\begingroup\$ The question has been rolled back to revision 1. This may or may not have consequences for the revisions to your answer. For your information. \$\endgroup\$
    – Mast
    Apr 24, 2020 at 10:18
0
\$\begingroup\$

There is a bug in the original code I posted. It was intended not to .filter() but to .takeWhile(). Only with that change the “funny fact” makes sence as RobAu pointed out. So the original code should have been:

import java.util.stream.IntStream;

class Leap {

    boolean isLeapYear(final int year) {
        return IntStream.of(4, 100, 400)
            .filter(divisor -> year % divisor == 0)
            .count() % 2 == 1;
    }

}

Incorporating the suggestions by @RobAu I would now change the code to the following to add more naming of variables and functions. I personally prefer them over adding comments, because comments tend to not being updated when the code changes.

import java.util.function.IntPredicate;
import java.util.stream.IntStream;

class Leap {

    boolean isLeapYear(final int year) {
        final IntPredicate yearFallsIntoCategory =
            category -> year % category == 0;

        final IntStream specialYears = IntStream.of(4, 100, 400);
        final IntStream categoriesYearFallsInto =
            specialYears.takeWhile(yearFallsIntoCategory);

        return categoriesYearFallsInto.count() % 2 == 1;
    }
}
```
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.