• some guy (unregistered)

    I suspect that the original developer saw some similarities between a person's birthday and the use case, found a bad birthday snippet, and used it.

  • LCrawford (unregistered)

    An off by one error is that they are declared as being one year old before their frist birthday. Variable 'now' not used. If run on their birthday, it adds an extra year since the reference time is midnight.

  • RLB (unregistered) in reply to some guy

    StackOverflow strikes again.

  • Anonymous (unregistered)

    I wonder if the simplified version will correctly handle leap years.

  • WTFGuy (unregistered)

    At least they used .Net's built-in DateTime class which means they don't need to special case the Feb 29 babies/cases. You know they'd have fouled that up if they'd done it stringly or in Java.

    As LCrawford says, bonus points for initializing and then not using the now var. Which means that if we're iterating around that while loop on midnight of the birthday we the result is dynamic. Said another way, if they used the now var the caller would get the last birthday as of the time of the method call(-ish) but as it's written the result is as of the time of the method return(-ish). At least the result isn't unstable or inverted.

  • WTFGuy (unregistered)

    IMO the off-by-one is more a matter of naming. The problem is that former & current are ambiguous words.

    If you consider them as meaning priorIteration and currentIteration relative to the while loop algorithm there's no error. The algorithm depends on overshooting by one iteration and then returning the priorIteration interim result as the final result.

    If instead you think of them as meaning priorToMostRecentBirthday and mostRecentBirthday then when the while gets to the overshoot condition and terminates, the outcome is that mostRecentBirthday is the one in the future and the priorToMostRecentBirthday is the one most immediately in the past.

    Variable naming is hard because there's always an embedded point of view. Are we global-centric and thinking about the birthdays in the flow of time or are we local/algorithm centric and thinking about the flow of the values inside the algorithm?

    IME the less skilled the developer the more they're totally focused at the make-my-chosen-algorithm-work level and are not thinking of the global problem in the global context. Which results in near-sighted naming. IME Christian's comment about sort-of- off-by-one is him seeing (like an optical illusion) the alternating interpretation of the variable names but not quite tumbling to the cause of his vague unease that's it's almost right despite passing any testing he might have done.

  • (nodebb)

    I think my ex-boss may have written this code.

    var now = DateTime.Now; ..... while (current < DateTime.Now)

    He did that all the time. I'm skeptical, though, because he would have set up all the variables and then created a loop that counts to an arbitrary value and come up with this:

    while (current < former.AddYears(1) { former = former.AddYears(i); current = current.AddYears(1); }

    return current.AddYears(-i); }

    Was always a joy working on his code.

  • Brian (unregistered) in reply to WTFGuy

    On the topic of naming things, "last" is a terrible word to use regarding an ongoing sequence. I don't know when my last birthday is, because hopefully I've got many more to go. The meaning is obvious in the context here, but I still prefer in general to use more specific names like "latest" or "most recent" to minimize any ambiguity.

  • Simon Clarkstone (unregistered) in reply to WTFGuy

    "they don't need to special case the Feb 29 babies/cases"

    I'd guess they do need to. If you start with Feb 29th and keep adding 1 year, you'll first get bumped to Mar 1st, then stay in Mar 1st for later years, even for years that have a Feb 29th that should be the birthday. Unless this date addition is more subtle than I realise.

  • MiserableOldGit (unregistered)

    We used to deal with the off by one errors quite a lot when I worked with pension scheme calculations. Different schemes and different rules were worded slightly differently as to whether something happened when you reached a date/anniversary, or once you were a certain age, or every day accrued after (including/excluding) etc. They were usually a bit vague and had been "clarified" by actuaries and then "amended" following class actions challenging the calculations. And that's before we get into cases where people had switched schemes with a guarantee to be "no worse off" or some correction was being retrospectively applied due to a prior mistake.

    Using a loop counting forward from one agreed day zero until "whatever" trigger event while accruing the days in relevant counters was the only way to do it and keep sane. Trying to implement the rule as an algorithm and then add and deduct all the exceptions until you get the right answer is a world of pain, I inherited a mess of code that was attempting to do it that way (and failing), a big name HPC firm was trying to replace me and my code with another single formula approach that was also a failure.

  • Shill (unregistered) in reply to Simon Clarkstone

    I suspect that there may be a business rule that says the case year start shall be Mar 1 for all Feb 29 birthdays, whether or not the current year is a leap year. If so the old code is correct and the new code is (likely) not.

    Further, I'm not sure what the big deal is about the name of the method. It does what it says it does. Just because the clients of the method use the last birthday in other contexts, doesn't make it a bad name. Suppose they named it calculateCaseYearStart(). Now they add a module concerning vacation accrual and the business rule is that vacation starts accruing on your last birthday. So vacationRestDate = calculateCaseYearStart(). Certainly not better than what we had before.

  • (nodebb)

    This definitely messes up with leap babies but there's another questionable one: If today is your birthday it returns today as your last birthday. I think most people would disagree with this.

    I also second the notion that this is recycled birthday logic.

  • Truism (unregistered)

    Honestly the whole article makes me feel a bit dated, but answering the easy reader especially.

  • Truism (unregistered)

    ...huh, a number on the first line causes alignment issues.

    I guess today's code snippet isn't the only thing with off-by-1 errors.

  • Chris (unregistered)

    And here was me thinking he was only 8 years old. Or 104.

  • Raj (unregistered) in reply to RLB

    Odds are that someone who copies bad code from StackOverflow would have written even worse code on their own. As long as we can agree that code quality is a spectrum, not a boolean, then StackOverflow has a positive effect even when bad code is posted.

  • QA (unregistered) in reply to Truism

    This is a peculiar bug.

  • QA (unregistered) in reply to QA

    1 Frist.

    Without a preview button it's hard to tell how much entertainment this bug can provide.

  • Stephen (unregistered) in reply to Simon Clarkstone

    "If you start with Feb 29th and keep adding 1 year, you'll first get bumped to Mar 1st, then stay in Mar 1st for later years, even for years that have a Feb 29th that should be the birthday"

    Oh, that's easy to fix. Just add an incrementing counter (or change the while to a for loop with an arbitrarily large upper limit) and add counter years to the original date to get the new current date. No need to throw this fine code away.

  • Hal (unregistered) in reply to Shill

    There is a word that covers all of this "anniversary"

  • gnasher729 (unregistered)

    Some people think code should be self-documenting. But in this case, your function needs to start with a big comment explaining exactly what should be calculated. For example, what does “next birthday” return on your birthday, today or next year? What if your birthday is Feb 29?

    I can figure out what code does, but that doesn’t help if I don’t know what it should do.

  • Wizofaus (unregistered) in reply to WTFGuy

    But that's not true - if you started on Feb 29, it would first increment to Feb 28, and then remain there even if the current year was a leap year.

  • (nodebb)

    Remy, you're 40?! But you look so young in your picture.

Leave a comment on “This is Your Last Birthday”

Log In or post as a guest

Replying to comment #515005:

« Return to Article