leap: Add years that refute some unusual factors

[petertseng]

leap 1.6.0

Some implementations unexpectedly pass the entire test suite:

• Replacing 4 with 499, 998, or 1996
• Replacing 100 with 5, 10, or 20
• Replacing 100 with 3, 6, 12, 15, 30, 60, 75, 150, 300
• Replacing 400 with 125, 250, 500, 1000, or 2000

Adding these test cases would correctly point out that these solutions
are incorrect.

I think it's a bit unusual since no student is going to write such an
implementation except a student deliberately trying to slip past the
tests.
Or maybe a student trying to micro-optimise? Maybe they are trying to
test whether division by small numbers is faster than division by large
numbers?

But discussion participants have deemed that the cost of three tests is
worth the benefit of reducing mental overhead of mentors, since such
solutions have in fact been seen in the wild.

[yawpitch]

Hmm ... actually, there's a point to be had here. Currently any of the below can be substituted for 100 and pass all the existing tests:

-300, -150, -100, -75, -60, -50, -30, -25, -20, -15, -12, -10, -6, -5, -3, 3, 5, 6, 10, 12, 15, 20, 25, 30, 50, 60, 75, 100, 150, 300

The -100, -50, -25, 25, 50, and 100 values are expected, since those are all factors of both 100 and 400, but the rest are a bit of a surprise.

Adding a test for 1960 gets rid 5, 10, and 20 (and their negative counterparts), which will give false negatives on every 20th year within a given century.

Adding a test for 1900 gets rid of 3, 6, 12, 15, 30, 60, 75, 150, and 300 (and their negatives), which will all gives false positives on years with both 4 and the multiple of 3 as a factor.

[petertseng]

Since we're in the vein of "replace an existing number with N and it still passes the tests", I have now also noticed:

• Replacing 4 with 499, 998, or 1996 will still pass all tests, since the only true cases are 2000 (covered by 400) and 1996. If we wished to prevent this, we would want to add one other true case (just not one divisible by 400). 1960 does fit this bill.
• Replacing 400 with 125, 250, 500, 1000, or 2000 will still pass all tests (for the same reason, just in the other direction). If we wished to prevent this, we need to add a different multiple of 400.

So, if we took everything that's been said into account, that would perhaps imply adding:

• 1960
• 1900
• 2400

I don't have any idea how likely a student is to actually write any of the implementations previously described, but it is an interesting thing to notice/discuss that test coverage is not an easy thing.

[yawpitch]

To be fair I’ve seen precisely one implementation that used 25 in place of 100... user thought division by smaller numbers would be faster. I don’t think the three additional cases would be much of a burden. M

…

On Sep 9, 2019, 03:46 +0100, Peter Tseng ***@***.***>, wrote: Since we're in the vein of "replace an existing number with N and it still passes the tests", I have now also noticed: • Replacing 4 with 499, 998, or 1996 will still pass all tests, since the only true cases are 2000 (covered by 400) and 1996. If we wished to prevent this, we would want to add one other true case (just not one divisible by 400). 1960 does fit this bill. • Replacing 400 with 125, 250, 500, 1000, or 2000 will still pass all tests (for the same reason, just in the other direction). If we wished to prevent this, we need to add a different multiple of 400. So, if we took everything that's been said into account, that would perhaps imply adding: • 1960 • 1900 • 2400 I don't have any idea how likely a student is to actually write any of the implementations previously described, but it is an interesting thing to notice/discuss that test coverage is not an easy thing. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[yawpitch]

After thinking about this for a bit I'm on the fence about re-opening and merging this; when it comes down to it this is one exercise where the correct algorithm is spelled out line by line ... if any number other than 4, 100, and 400 appears in the resulting implementation someone's already strayed far enough off the narrow path of logic to require manual intervention from a mentor.

[sshine]

I’ve seen precisely one implementation that used 25 in place of 100

if any number other than 4, 100, and 400 appears in the resulting implementation someone's already strayed far enough off the narrow path of logic to require manual intervention from a mentor.

Weirdly enough, I've seen 3-4 on Haskell.

I think people are trying to be smart and cryptical with modular arithmetic.

If adding better coverage by adding three unit tests can reduce the mental overhead from mentors, so they're not forced to say as often "I have no idea if this actually works; it's not obvious that this solution is equivalent to the trivial solution with the constants 4, 100 and 400, so all creativity is on your expense." I'd be in favor of that. :-)

Wrt. years that are past the point in history where leap years were introduced: These are outside the domain of the leap-year function and so any implementation should either restrict the input (impractical), restrict the output (error), or accept some kind of undefined behavior. Any of these could be deemed acceptable.

[yawpitch]

Let’s add the three tests then; it won’t effect most people, and will dissuade creative, but nevertheless wrong, exploration.

As for years in other calendar, we can just ignore those; this algorithm applies to all years — if you assume Pope Gregory was in fact infallible about the length of a year — even ones that were never marked as leap years in the calendar then in use (aka the proleptic Gregorian years), so long as you use astronomical dating.

Actually identifying a leap year in any calendar would require a solution many thousands of LoC long, and access to an accurate database that doesn’t exist plus the geographical position in which the request was being made.

[kotp]

An example of a potentially trivial exercise that given religion and politics and a man made thing applied to the physical is anything but trivial.

[yawpitch]

I have a feeling that once you actually understand Haskell that is in fact trivial.

…

On Sep 10, 2019, 14:28 +0100, Victor Goff ***@***.***>, wrote: An example of a potentially trivial exercise that given religion and politics and a man made thing applied to the physical is anything but trivial. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[kotp]

I will usually leave it to those that have done the research and use the language's Date facility. I don't want to know when Greece adopted the Gregorian calendar in 1923, or that September 1752 looks like this:

   September 1752     
Su Mo Tu We Th Fr Sa  
       1  2 14 15 16  
17 18 19 20 21 22 23  
24 25 26 27 28 29 30  

[petertseng]

Very well. Here are the best test descriptors I could come up for these tests. Hopefully they don't draw too many "what? where did 125 come from here?" moments.

By the direction of the conversation, it seems like the discussion participants are classifying this as a bug fix, since for most students it doesn't materially change what's required of the student; it's mainly for preemptively answering some "can I change this number and still have it work?" moments. Very well then.

[sshine]

I will usually leave it to those that have done the research and use the language's Date facility.

This is why my favourite solution in Haskell looks like

module LeapYear (isLeapYear) where
import Data.Time.Calendar (isLeapYear)

:-D

[yawpitch]

An occasional solution in Python is :

from calendar import isleap as leap_year

Which ultimately does return year % 4 == 0 and (year % 100 != 0 or year % 400 == 0)... so not something that ever touches a more elaborate library that can handle really odd cases.

But then neither does Data.Time.Calendar:

isLeapYear :: Integer -> Bool
isLeapYear year = (mod year 4 == 0) && ((mod year 400 == 0) || not (mod year 100 == 0))

Though that seems to be less efficient than the Python approach, assuming that && and || are short circuiting.

[sshine]

@coriolinus, @kotp: If you are waiting for this PR in exercism/rust#871, feel free to review this one. :-)

[coriolinus]

These tests look good to me. However, I have not personally verified that they rule out all the malicious examples used in @petertseng 's original statement.