• (nodebb)

    (though my preference would be to just return the expression instead of using a local variable)

    Mine as well, although doing it the way it is in the code may be a relic of early debugging, allowing the programmer to set a breakpoint to allow verification of the result. (And even a halfway-decent(1) optimiser will remove the redundant load implied by the separate return statement, and then remove the now-redundant store as well.)

    (1) I have seen a C compiler whose "most aggressive" (the words in the compiler's docs) optimisation mode wouldn't have been able to do the second, and probably not the first either. Heck, it couldn't refactor the assembly for "while(1)" to remove the comparison to see if 1 was not 0.

  • (nodebb)

    I'm surprised they didn't do the normal new DateTime(dt.Year, dt.Month, 1).AddMonth().AddDay(-1). I actually don't know a method for LastDayOfMonth, honestly never needed it too.

  • Hanzito (unregistered)

    There even is a "DaysInMonth(year, month)" method, apparently since NET 1.0. But the story isn't very thrilling. It's probably: port this method to C#. And since copy-paste almost works, that's what they did. "We'll come back to it later, when we have more time," they promised. Time is one of the monsters, sure, but perfection is the other.

  • (nodebb)

    I saw the title of this story in my RSS feed and thought "all the rest have 31": that's a quote isn't it", but I couldn't figure out where from. It's only now I've read the story and all the comments that it twigged.

  • y2k16 (unregistered)

    int months = 0x0a52; int years = 0x8808; year -= 1993 day -= 30; if (months & (1 << month)) else day -= 1 /* compiles to BT + conditional DEC */ dec = month == 2 day -= ((years & (1 << year)) ? dec : 0) day += dec; day += dec; return day == 0;

    No jumps, no modulos, no libraries, no syntactic opium

  • NobodySpecific (unregistered)

    So... the WTF in this case is that they did not create bugs or authored bizarre behaviors that return completely wrong results when they wrote functions that were never needed in the first place? Eh, there's room for improvement but as far as WTFs go I'd trade any of the ones I experienced in my career for this one.

    Although, while the "horror" in this codebase is mild, there is potential for true horror in the management layer given that the developers are apparently tasked with "rushing ported code first, refactor never". But we did not get many details in that front.

    It's like we're watching a horror movie that stops before the murderer had a chance to do anything onscreen other than show his hand slowly pushing open the cabin's door.

  • (nodebb) in reply to Hanzito

    Ha, I never had to use that one in over two decades. But good to know.

  • Darren (unregistered)

    While there are usually built-in functions for these things, the built-ins are probably just running the same code as is written here. So, no harm, no foul.

  • Lurk (unregistered)

    As others have said we don't know why this was left in place, but it smells like the end result of a series of "minimum effort, minimum risk" ports from one language to the next with a liberal garnish of "Ain't broke, don't fix it."

    Leaving the code aside the bit I really like is the pointless summary headers on the methods. What're the odds there's a coding "standard" (spit!) in place that says all methods have to have such things?

  • (nodebb) in reply to Darren

    Actually, not really.

    While IsLeapYear is a slightly optimized version of the method presented in the article, DaysInMonth is a simple lookup in two static arrays, so super fast in comparison to the solution in the article.

    .net is multi-platform, open-source and non-proprietary, so you can check the actual code here:

    https://github.com/dotnet/runtime/blob/5535e31a712343a63f5d7d796cd874e563e5ac14/src/libraries/System.Private.CoreLib/src/System/DateTime.cs#L1171

  • Randal L. Schwartz (github)

    In Dart, you can use the built-in "wraparound" logic: DateTime(year, month+1, 0)), which will give you the last day of year/month... ALWAYS.

  • BZ (unregistered)

    It's rare to get everything correct. At my first job we had a similar routine, but over time two serious mistakes were found:

    1. Somehow 4 was missing from the switch statement, meaning that April had zero days. Amazingly, it took a while for this to be discovered at all given division by zero errors it produced.
    2. February returned 29 days, Since the code was checked in during February of a leap year, no errors were reported until almost a year later.
  • (nodebb) in reply to y2k16

    Trying to make sense of your code, which I have reformatted below.

    int months = 0x0a52;
int years = 0x8808;
year -= 1993
day -= 30;
if (months & (1 << month))
else day -= 1
/* compiles to BT + conditional DEC */
dec = month == 2
day -= ((years & (1 << year)) ? dec : 0)
day += dec;
day += dec;
return day == 0;

    The if statement is also missing a body. Is day -= 1 supposed to be the body, or was there really meant to be an else clause? If I try it either way (if body is a no-op, or day -= 1 is the body), I get different results depending on what day of the month month (singular) is set to, . How is this code meant to work?

  • some guy (unregistered)

    Noone commented on the doc comments, which point to the real horror: someone made them add the useless skeleton (sic), and they did not fight back or even paid lip service to the documentation. "We have met the enemy and he is us."

  • (nodebb) in reply to y2k16

    You have an if and a ternary in there. that's not no jumps.

  • (nodebb)

    Every time I read one of this all I can think of is Tom Scott loosing his s**t in Computerphile's "The Problem with Time and Timezones" video. :D

  • Loren Pechtel (unregistered)

    This doesn't belong here. It's just a simple port from a system that didn't have it in the library. I'm not even going to gripe about not using a default because on something like this I would prefer clarity over brevity.

  • (nodebb)

    It makes sense to let the people writing the API for the language come up with the most efficient and well-tested way to solve general problems like this. Your solution may work, but I would trust the solution provided by the DateTime class to be at least as good as, if not better than what we come up with. It may not make much difference, but it means one less place you need to maintain, one less place for a weird edge case to creep in, and an overall tidier codebase. Plus there's the very mild benefit where, for example, future CPUs have built in hardware functions for date / time calculations, and the latest C# codebase takes advantage of it. If there's no extra cost, let the professionals provide your wheels for you.

  • LZ79LRU (unregistered)

    I find it highly probable that the people porting this did not know that C# had a DateTime class to begin with and were just copy pasting C. Someone on the team was probably given the choice between C# and Java and they picked C# on the count that it's "all the same" since it's C.

  • (nodebb)

    Actually there is a bug in the LeapYear code. The only reason it did not trigger during Y2K has to downing how leapyears work. The first time it triggers is in the year 2100.

    Leap years are every 4 years except when it is a full century then it’s not. But there is an exception to that too, when the year is divisible by 400 (with out a remainder) then it is still a leap year.

    Addendum 2024-11-01 04:25: Downing is “do with how” DYAC.

  • y2k16 (unregistered) in reply to gordonfish

    thosrtanner A ternary can be interpreted as a hint to the compiler to generate a cmov, instead of a branch.

    GordonFish, that's the point: test whether day is the end of the month, depending on which month it is.

    How does it work? There is a look-up table for leap years since 1993 in 0x8808, and another look-up table for less-than-31-day months in 0x0a52.

  • (nodebb) in reply to y2k16

    That's fine, but results I am getting trying to use your code don't match your description. It feels like there are missing bits. Like I said, the if statement has no body, just immediately an else. And seemingly missing semi-colons. What language is this supposed to be? My first thought was C but I'm not quite sure. Could you provide working code and what input you tested it with?

Leave a comment on “All the Rest Have 31”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article