GetTimeNow

mallard

Been working on a date related bug in a monitoring app that I'm maintaining (dates being passed around as strings, different code assumes different formats, when the date is ambiguous bad things happen) and came across this:

    Function getTimeNow()

        Dim hour As String = String.Empty
        Dim min As String = String.Empty

        'set the hour in the correct format
        If Date.Now.Hour < 10 Then
            'add a 0 in the front
            hour = "0" & Date.Now.Hour
        Else
            hour = Date.Now.Hour
        End If

        'set the min in the correct format
        If Date.Now.Minute < 10 Then
            'add a 0 in the front
            min = "0" & Date.Now.Minute
        Else
            min = Date.Now.Minute
        End If

        Return hour & ":" & min & ":00"

    End Function

Any reason why this is better than Date.Now.ToString("hh:mm") & ":00" ? None that I can see... good thing this function is never called then...

IanB

I don't know the details, but maybe one format goes from 1 to 12 and the other from 0 to 23?

You need to be careful if you keep polling the current time, if the hour or minute increment mid-function you could get e.g. 010:00:00 when hour goes from 9 to 10, or 05:010:00, when minute goes from 9 to 10, or 04:00:00 at either side of 5 o'clock

Better to store Date.Now at the start and work on that.

Ian.

DOA

There's a very good reason for it. I believe it's called "Not Invented Here". Why use a single line of code when you can waste hours creating drivel like this?

blupp

I always "like" the way these guys comparing a volatile variable with a constant value and then conditionally use the volatile variable:

        If Date.Now.Minute < 10 Then
            'add a 0 in the front
            min = "0" & Date.Now.Minute
        Else
            min = Date.Now.Minute
        End If

should be

        minutes = Date.Now.Minute
        If minutes < 10 Then
            'add a 0 in the front
            min = "0" & minutes
        Else
            min = minutes
        End If

so the Date.Now.Minute does not change between the test for < 10 and the next statement where it is read again. okok, the window of opportunity is really small, but it is there.

but the best solution IMHO is still this:

minutes = Date.Now.Minute
min = right$("0" & minutes,2)

djork

Are you sure we're not working on the same project? The only thing that would indicate otherwise is the lack of a fully-commented Try, (empty) Catch, (empty) Finally construct.

pitchingchris

This vb code is hurting my eyes !

jcoehoorn

"This vb code is hurting my eyes !"

The appearance of the code is remarkably sane for vb code. Variables are declared with types, and the if/else structure is very readable. The fact that they want to encapsulate the functionality provided by Date.Now.ToString("hh:mm") & ":00" isn't a big deal either. If you do this a lot it's nice to have that small bit a logic scurried away somewhere. But the fact that they took 20 lines to implement a one-liner, the volatile variable bug, and the fact that the function exists but is never used make this wtf-worthy.

A reason why it is worse.

If Date.Now.Hour < 10 Then
'add a 0 in the front
hour = "0" & Date.Now.Hour

When the first Date.Now.Hour returns 9:59:59.999999999 then the result will be 010:00:00.

Unlikely, but not impossible.

JvdL

@blupp said:

I always "like" the way these guys comparing a volatile variable with a constant value and then conditionally use the volatile variable:
        If Date.Now.Minute < 10 Then
            'add a 0 in the front
            min = "0" & Date.Now.Minute
        Else
            min = Date.Now.Minute
        End If
should be
        minutes = Date.Now.Minute
        If minutes < 10 Then

            'add a 0 in the front

            min = "0" & minutes

        Else

            min = minutes

        End If

so the Date.Now.Minute does not change between the test for < 10 and the next statement where it is read again. okok, the window of opportunity is really small, but it is there.

I "like" how your solution has the same problem as the original.

hour = Date.Now.Hour 'Yields 5 on 5:59:59.999

minute = Date.Now.Minute 'Yields 0 on 6:00:00.000

blupp

@JvdL said:

I "like" how your solution has the same problem as the original.
hour = Date.Now.Hour 'Yields 5 on 5:59:59.999
minute = Date.Now.Minute 'Yields 0 on 6:00:00.000

And you are right indeed! but it was good enough for an example :-)

aib

@jcoehoorn said:

The appearance of the code is remarkably sane for vb code. Variables are declared with types, and the if/else structure is very readable. The fact that they want to encapsulate the functionality provided by Date.Now.ToString("hh:mm") & ":00" isn't a big deal either. If you do this a lot it's nice to have that small bit a logic scurried away somewhere. But the fact that they took 20 lines to implement a one-liner, the volatile variable bug, and the fact that the function exists but is never used make this wtf-worthy.

Can't you do Date.Now.ToString("hh00") ?