4
\$\begingroup\$

I'm using DateTime C#. I have an XML file with attribute HOUR, i.e:

<MyXML>
     <LastTimeTaskRun>11:50</LastTimeTaskRun>
</MyXML>

And i have a task that running each 5 min. I want to know if the current time and the hour from the xml big then 5 min. What I'm trying:

int hour, min;
DateTime dateTimeXML = DateTime.Now, dateTimeNow;
string [] lastSuccessTime = LastSuccessTime.Split(':');

Int32.TryParse(lastSuccessTime[0], out hour);
Int32.TryParse(lastSuccessTime[1], out min);

dateTimeXML = dateTimeXML.Date + new TimeSpan(hour, min, 0);
dateTimeNow = DateTime.Now;

bool isBigThen5Min = (dateTimeNow - dateTimeXML).TotalMinutes > 5;

The code is works fine, but i want to know what is the best way (performance) to do that.

\$\endgroup\$
1
  • 2
    \$\begingroup\$ If LastTimeTaskRun is 23:59 and then you run at 00:02 the next day your code will incorrectly calculate a time difference greater than 5 minutes (when it's only 3). \$\endgroup\$ – RobH Jun 22 '16 at 9:53
4
\$\begingroup\$

Short of following Heslacher's advice on a Timer, see RobH's comment regarding crossing dates. Then extend that thinking to any daylight saving transitions. You might say you are never going to run the app during the middle of the night, but in the future this app could be a service or a scheduled task that does run near midnight or 2 AM.

For internal timings, one should always prefer DateTime.UtcNow over DateTime.Now for 2 reasons: (1) its faster and (2) its not error prone to DST transitions.

You may think in terms of just time for your XML, but you would be safer and using better practices if you passed an ISO 8601 compliant string. This is easy to do with .NET by using the Round-trip "O" Format Specifier. Strongly recommend you carefully read the link on Round-trip.

This simplifies your coding while making the app less error prone. Assuming the variable LastSuccessTime is a time string as read from the XML, you no longer have to take a lot of steps to parse out pieces of it. Your XML would look like:

<MyXML>
     <LastTimeTaskRun>2016-06-21T23:59:00Z</LastTimeTaskRun>
</MyXML>

Such that LastSuccessTime would contain "2016-06-21T23:59:00Z".

Now you could use code like:

var lastSuccessUtc = DateTime.Parse(LastSuccessTime, null, DateTimeStyles.RoundtripKind).ToUniversalTime();

var isBigThen5Min = (DateTime.UtcNow - lastSuccessUtc).TotalMinutes > 5;

The last line contains a magic number, so you should consider making a constant for it. You can also get the same effect different ways (shown for example, not necessarily a better practice):

const int thresholdInMinutes = 5;
var isBigThen5Min = (DateTime.UtcNow - lastSuccessUtc) > TimeSpan.FromMinutes(tresholdInMinutes);

This answer would work across midnight across different dates and for DST transitions. Plus it shortens up the code. Shorter and simpler is easier to follow and maintain.

\$\endgroup\$
2
  • 3
    \$\begingroup\$ Since the round-trip format is also the format for dates in XML, OP could use LINQ-to-XML to read/write the date rather than DateTime.Parse: var element = XElement.Parse(xml); var date = (DateTime)element; It will automatically figure out the date is UTC because of the Z specifier at the end. \$\endgroup\$ – Dan Lyons Jun 23 '16 at 17:32
  • \$\begingroup\$ Great tip, @DanLyons. Didn't know that myself. \$\endgroup\$ – Rick Davin Jun 23 '16 at 18:24
2
\$\begingroup\$

For parsing the time you can use the ParseExact method:

var lastTimeTaskRun = DateTime.ParseExact("11:23", "HH:mm", CultureInfo.InvariantCulture);

To calc the difference you can directly subtract now and the last time:

var isGreaterThen5Min = (DateTime.Now - lastTimeTaskRun).TotalMinutes > 5;

It'd be also a good idea to define the 5 as a const:

const int maxIntervalInMinutes = 5;
var isGreaterThenMaxInterval = 
    (DateTime.Now - lastTimeTaskRun).TotalMinutes > maxIntervalInMinutes;

You should stay consistant with the variable names. The xml element is named LastTimeTaskRun but the property is LastSuccessTime. Pick one and use it everywhere. Otherwise you'll need to explain and think all the time what was it for.

\$\endgroup\$
2
\$\begingroup\$

Above other answers.

  1. Have you considered storing the time in the Project Settings? They are easy to read, support types (i.e. can store DateTime objects) and are generally simpler to deal with.
  2. To improve readability you should perhaps write one var declaration per line:

    DateTime dateTimeXML = DateTime.Now;
    DateTime dateTimeNow;
    

    This is what MS recommends. And this immediately discloses violating the good principle of initializing variables simultaneously with declaring whenever possible. So instead of

    DateTime dateTimeNow;
    dateTimeNow = DateTime.Now;
    

    you'd better write

    var dateTimeNow = DateTime.Now;
    

    Also these considerations might be interesting.

  3. As for the name for the isBigThen5Min variable, I nearly like @t3chb0t 's suggestion, only change thEn to thAn: isGreaterThanMaxInterval.

Also, it looks like you haven't really dealt with date and time in your programming life, so here is some fun reading to get inspired: falsehoods programmers believe about time :)

\$\endgroup\$
1
\$\begingroup\$

The easiest way to do this would be using a Timer.

See for an example : https://stackoverflow.com/questions/12535722/what-is-the-best-way-to-implement-a-timer

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.