Minor fail with date comparison



  • I found this (anonymized) method in our production code:

    //inputDate and anchorDate are converted to local time with the time portion stripped off
    boolean isNotPrior(DateTime inputDate, DateTime anchorDate)
    {
    return inputDate.plus(Seconds.ONE).isAfter(anchorDate);
    }

    I would have thought it would be simpler and more accurate to just use return !inputDate.isBefore(anchorDate);.
    Also, in our codebase, instead of a parallel isNotSubsequent() method, this method is re-used with the passed parameters switched around.


  • Discourse touched me in a no-no place

    @djls45 said in Minor fail with date comparison:

    Also, in our codebase, instead of a parallel isNotSubsequent() method, this method is re-used with the passed parameters switched around.

    Oh great, all that extra complexity for someone to comprehend. Bonus points if isAfter, isNotPrior and isNotSubsequent are all used in the same class somewhere. Triple bonus if they're in the same expression…



  • @dkf said in Minor fail with date comparison:

    @djls45 said in Minor fail with date comparison:

    Also, in our codebase, instead of a parallel isNotSubsequent() method, this method is re-used with the passed parameters switched around.

    Oh great, all that extra complexity for someone to comprehend. Bonus points if isAfter, isNotPrior and isNotSubsequent are all used in the same class somewhere. Triple bonus if they're in the same expression…

    Here is another small nugget from the same class:

    boolean isWithinRange(DateTime inputDate, DateTime anchorDate, DayRange days)
    {
    boolean result = true;
    DateTime testDate = anchorDate.toDateMidnight().toDateTime();
    if(days.getBefore() != null)
    {
    DateTime beforeDate = testDate.minusDays(days.getBefore()).minus(Seconds.ONE);
    result = inputDate.isAfter(beforeDate);
    }
    if(days.getAfter() != null)
    {
    DateTime afterDate = testDate.plusDays(days.getAfter() + 1);
    result = result && inputDate.isBefore(afterDate);
    }
    return result;
    }
    It does use both isBefore() and isAfter() from DateTime, but not isNotPrior(). I'm not sure why there's so much manipulation of the dates for the comparisons, when they are passed in as midnight local time in all uses. (Notice the superfluous conversion to midnight on the second line.) Perhaps it's due to code creep, with no refactoring for efficiency/clarity going on...?



  • @djls45 said in Minor fail with date comparison:

    Seconds.ONE

    Do you also have Seconds.FIFTY_TWO_AND_THREE_QUARTERS?



  • What's with the names: isNotPrior(), isNotSubsequent()

    Someone into not logic? I mean, think about it:

       if (!isNotPrior(date1,date2))
       {
            ...
       }
    

    Even I was able to come up with better names than that: precedes(), succeeds()



  • @dkf said in Minor fail with date comparison:

    Bonus points if isAfter, isNotPrior and isNotSubsequent are all used in the same class somewhere. Triple bonus if they're in the same expression…

    Quadruple if they're all implemented differently....

    @CoyneTheDup said in Minor fail with date comparison:

    Even I was able to come up with better names than that: precedes(), succeeds()

    proceeds(), concedes()?



  • @PJH said in Minor fail with date comparison:

    @CoyneTheDup said in Minor fail with date comparison:

    Even I was able to come up with better names than that: precedes(), succeeds()

    proceeds(), concedes()?

    For date comparisons? Not likely.

    Also, you forgot: concedes(), intercedes(), secedes(), recedes(), Mercedes()
    Every date utility class needs all of those, right?



  • @CoyneTheDup said in Minor fail with date comparison:

    What's with the names: isNotPrior(), isNotSubsequent()

    I came up with isNotSubsequent() to go with isNotPrior(). It's not actually in this class.

    Someone into not logic? I mean, think about it:

       if (!isNotPrior(date1,date2))
       {
            ...
       }
    

    In this case, the code uses if(isNotPrior(date2,date1)).

    Even I was able to come up with better names than that: precedes(), succeeds()

    The proper usage would be to use the built-in functionality of DateTime:
    date1.isBefore(date2), date1.isAfter(date2), !date1.isBefore(date2), !date1.isAfter(date2).


  • Discourse touched me in a no-no place

    @CoyneTheDup said in Minor fail with date comparison:

    Also, you forgot: […] Mercedes()
    Every date utility class needs all of those, right?

    Yes. undefined


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.