• Little Bobby Tables (unregistered)

    So what was it before the changes, and what would Richard have preferred?

  • MDK (unregistered) in reply to Little Bobby Tables

    Before the changes, the code was presumably the same, only the obnoxious comments were added. And as to what Richard would have preferred, possibly something like this (assuming that the *_int variables are used at a later stage):

    from datetime import datetime as dt
# ...
date_int = dt.now().day
hour_int = dt.now().hour
  • Max Chase (google)

    Starting from

    now = datetime.datetime.now()

    I would say

    day = now.day
hour = now.hour

    But sure we could call them date_int and hour_int.

    Ugh I haven't commented in forever and I don't know if that's going to come out at code or quotes or nothing or something really surprising.

  • MDK (unregistered) in reply to Max Chase

    Yes, fair, however as the code is seemingly only interested in the day and the hour, storing now() in a variable seems unnecessary as well.

  • Richard (unregistered)

    In all fairness. "Make it simpler, it should be simple" is a pretty terrible code review comment

  • The Zorro-probability (unregistered) in reply to MDK

    In a very unlikely case, you could switch day between the two calls. If you manage to encounter that case, remember to buy lottery tickets too :D

  • (nodebb)

    As a result, Klaus was fired and now has a :((((

    Just kidding. He was promoted to senior management and now defines coding guidelines.

  • Ebbe Kristensen (google) in reply to MDK

    In my opinion the real WTF is Richard's lack of code reviewing skills.

    Saying "the code is too complicated" is not a proper review comment. Instead say "Consider using the hour attribute of the datetime class to get the hour value". This would have saved a lot of time - and taught Klaus something he obviously did not know.

  • (nodebb)

    TRWTF is that he does the day using string parsing, but the hour directly. He still casts an integer to an integer for hour, but I suppose it's better than string roundabout?

  • Hasseman (unregistered) in reply to Ebbe Kristensen

    Thank you for that comment!!!!

  • Scott (unregistered)

    I'll go from the assumption that Klaus is not a fresh-out-of-school person with no experience. If so, how does this not end in immediate termination?

    These are the basics of the language, not some esoteric aspect that the reviewer happened to know another approach.

    In other words, the kind of shit I have to deal with every day from the consultants we have laying around.

  • siciac (unregistered) in reply to MDK 
    date_int = dt.now().day
hour_int = dt.now().hour

    Seriously?

  • (nodebb) in reply to The Zorro-probability

    Why buy lottery tickets? You have already used up your local allocation of improbability just splitting days between function calls.

  • Stranger Things (unregistered)

    I am wondering about this point-- if the dt.now().day executed at just the right moment before midnight and dt.now().hour executed at just the right moment after midnight would that produce results for yesterday as the day and today as the hour? Once fixed such a thing in code that read logged information and made daily reports. It was billing data for bandwidth or some such usage. As this was C-code, it was easier to fix. Read the current time to a variable only once for the scope of the function.

  • Anon E Mouse (unregistered) in reply to MDK

    Once upon a time, successive calls to now() didn't always return the same value, so GreyBeards typically make one call and save the value, knowing it gets optimized "properly"

  • RLB (unregistered) in reply to Anon E Mouse

    @Mouse They will never always return the same value, unless you know something about the cessation of the rotation of the Earth (or the disappearance of the Sun) that the rest of us don't.

    Granted, the luck required to hit that precise moment will grow as CPU ticks get shorter, but it'll never grow to infinity, let alone beyond.

  • Herr Otto Flick (unregistered) in reply to Scott
    I'll go from the assumption that Klaus is not a fresh-out-of-school person with no experience. If so, how does this not end in immediate termination?

    Its difficult to find competent developers, so you tend to give people quite a few go's to become competent. I'd certainly be rejecting that review until he gets it right, and flag it to his line manager as "Klaus seriously needs to get better, and fast."

    Once upon a time, successive calls to now() didn't always return the same value, so GreyBeards typically make one call and save the value, knowing it gets optimized "properly"

    Once upon a time, gettimeofday(2) was expensive, and you didn't just go around re-fetching it constantly. Would you fetch a row from the database each time you want a different column in one function? Same thing.

  • Richard Brantley (unregistered) in reply to Ebbe Kristensen

    "Saying 'the code is too complicated' is not a proper review comment."

    I have to disagree here. The lesson of this code review is not how to do dates better, but that the code was circuitous when it didn't need to be. A few minutes digging through python's date library would have made this a non-issue before the start; but the programmer assumed he was brilliant and the code reviewer was stupid, and that's the real problem. Explaining basic date manipulation to the coder solves the symptom but not the problem.

  • (nodebb) in reply to Ebbe Kristensen

    "the code is more complicated than it needs to be"

  • (nodebb) in reply to Herr Otto Flick

    Its difficult to find competent developers

    Yes, but Klaus's primary competence appears to be "condescending insulting fucknut", which is, indeed, not equivalent to "competent developer".

  • sizer99 (google) in reply to MDK

    The nice thing about making the snapshot variable, besides avoiding any rollover inconsistency between calls, is it also gives you a convenient handle. Why proliferate unrelated variables? You don't need date_int and hour_int if you've got 'now = dt.now()' and can just refer to 'now.day' and 'now.hour'. So it's got temporal consistency and naming consistency.

  • Anon (unregistered) in reply to The Zorro-probability

    I've dealt with enough race conditions to know that if you run a particular block of code frequently enough, you will eventually lose the race. Or in your terms "win the lottery".

    In addition, when splitting up components of dates, always use a snapshot variable. Ideally a negligible time elapses between reading day and hour (or hour and minute etc.). But under heavy load, another process may get priority and a relatively large amount of time could elapse. Perhaps the chance of experiencing a day/hour inconsistency is low, but hour/minute and minute/second inconsistencies are much more likely. So practice good code hygiene and use a snapshot variable in all cases.

  • Your Name (unregistered)

    FIRE ALL PROGRAMMERS that write “clever” code, or any code they claim that you must be clever to understand it. Load shrapnel into the cannon before stuffing the respective programmer in. Don't offer a tin pot for head protection.

  • WTFGuy (unregistered)

    The other reason to use a snapshot variable is for testability. How are you going to write unit tests over date/time related code that will deliver repeatable results if you can't inject the date/time as a dependency?

    A quick search over [dependency injection datetime] will get you plenty of discussion on this topic.

  • Ulysses (unregistered)

    TRWTF is the phrase 'parse dates out to strings.'

  • an0n (unregistered) in reply to Max Chase

    initially read "Ugh I haven't commented in forever" as "I don't write comments in code", assumed you work where I do.

  • xtal256 (unregistered) in reply to Richard

    "In all fairness. "Make it simpler, it should be simple" is a pretty terrible code review comment"

    Not when you think you're dealing with someone that has a brain. Unfortunately, many programmers are clueless and need everything spelled out for them (much like the computers they code for).

  • Barf4Eva (unregistered) in reply to Scott

    Damn dude... I mean, I kinda agree with you, but sometimes, even if the reasons are unreasonable, you give a second chance (or third) and find out they end up being capable. Agree w/ what another dev said... Can't always find good devs, so you take the ones you got...

    Like this guy we had that associated everything by an index of an array... with the many other arrays... in his single class. OOP - Oblivious Object Programming..? :(

    Ok, on second thought, immediate termination...

  • (nodebb)

    Now is the hour, when we must say goodbye...:)))))

  • David (unregistered)

    Indeed, to "complicated" for me:

    int("22:36") Traceback (most recent call last): File "<stdin>", line 1, in <module> ValueError: invalid literal for int() with base 10: '22:36'

    Luckily now.hour already returns an int with the our, not a string like "22:36" as the comment seem to imply. So int(hour) is just a nop.

  • Gnasher729 (unregistered) in reply to MDK

    Calling dt.now() twice will once in a while produce rubbish when the first call is just before and the second call just after midnight.

  • Klaus (unregistered) in reply to Richard

    Richard, My point exactly!

  • Sam (unregistered)

    "As is the case with pretty much any language these days, Python comes with robust date handling functionality."

    Except, of course, Javascript.

  • Quirkafleeg (unregistered) in reply to Stranger Things

    Precisely right. Anyone who has produced code for customers knows that the "that'll never happen" scenario is usually the first thing reported by the first customer... and such thoughts usually break Rule #1 in the Good Programmers Guidebook: "Assumption is mother of all f*** ups" (Re: Under Siege 2)

Leave a comment on “What a Happy Date”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article