HiveBrain v1.2.0
Get Started
← Back to all entries
principleMinor

Checking for a date and compare 2 different files

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
checkingdatedifferentfilesforandcompare

Problem

From snippets around the web, I was able to put together this .bat file to check for a date and compare 2 different files.

This works below but I want to clean it up so it can be much lighter and faster and MUCH less code.

As you can see, it uses the same code to check date with hour and number. It is basically checking to see if the .xml file is older than 15 minutes and then will download a new version if over 15 minutes.

```
@echo off
cls
setlocal EnableDelayedExpansion

SET execTime=15

REM lets get XML date
SET xml=my.xml
FOR %%i IN (%xml%) DO SET fileDate=%%~ti
SET timeString=%fileDate%

:: Get the "AM PM" string to check the HOUR interval
FOR /F "tokens=1-3 delims= " %%A IN ('echo %timeString%') DO SET AM_PM=%%C

:: first split so can delele the date
FOR /F "tokens=1-5 delims= " %%A IN ('echo %timeString%') DO SET theDate=%%A& SET HOUR=%%B

:: fix up the date to only numeric
set fixDate=%theDate%
set xmlDate=%fixDate: =%
set xmlDate=%fixDate:/=%

::split by ":" using "HOUR" set above
FOR /F "tokens=1-5 delims=:" %%A IN ('echo %HOUR%') DO SET H_NUM=%%A& SET M_NUM=%%B

:: Set HOUR intervals to "24 hour" format
IF %AM_PM%==PM (
IF %H_NUM%==01 (SET H_NUM=13)
IF %H_NUM%==02 (SET H_NUM=14)
IF %H_NUM%==03 (SET H_NUM=15)
IF %H_NUM%==04 (SET H_NUM=16)
IF %H_NUM%==05 (SET H_NUM=17)
IF %H_NUM%==06 (SET H_NUM=18)
IF %H_NUM%==07 (SET H_NUM=19)
IF %H_NUM%==08 (SET H_NUM=20)
IF %H_NUM%==09 (SET H_NUM=21)
IF %H_NUM%==10 (SET H_NUM=22)
IF %H_NUM%==11 (SET H_NUM=23)
) ELSE (
IF %H_NUM%==12 (SET H_NUM=12)
IF %H_NUM%==1 (SET H_NUM=01)
IF %H_NUM%==2 (SET H_NUM=02)
IF %H_NUM%==3 (SET H_NUM=03)
IF %H_NUM%==4 (SET H_NUM=04)
IF %H_NUM%==5 (SET H_NUM=05)
IF %H_NUM%==6 (SET H_NUM=06)
IF %H_NUM%==7 (SET H_NUM=07)
IF %H_NUM%==8 (SET H_NUM=08)
IF %H_NUM%==9 (SET H_NUM=09)
)

SET TIMESTAMP=%H_NUM%%M_NUM%
SET timeXmlCreated=%TIMESTAMP%

REM >>

echo/|set /p =%date% >time.bat
time /t >>time.bat

SET mktime=time.bat
FOR %%i IN (%mktime%) DO SET timeFile=%%~ti
SET timeString=%timeFile%

Solution

General issues:

1) You should learn how to create and call batch functions so you have re-usable code. See the batch function tutorial at DosTips.com. (Note that the name of the site is misleading. It is primarily a Windows batch site, not DOS.)

2) You put the current date and time in the temporary "time.bat" file, but you never use that data. Instead you use the last modifed timestamp of the newly created file to get the current timestamp. You could have used the dynamic %TIME% and %DATE% "variables" to get the current timestamp without using any temp file. However, there is a much better way to get the current timestamp.

3) Your temporary file has a .bat extension, so it could be executed (with failure of course, since it does not contain valid batch commands). If you create a temp file to house data, then it should have a benign extension. Often times .tmp is used. But again, you really don't need a temp file at all.

4) There is no guarantee that the user has write access to the current directory. When creating temp files, it is best to place them in the "%temp%" folder. This variable is preset for each user to point to a safe place to write temporary data.

5) You use the "time.bat" literal repeatedly, but then you define mktime and use %mktime% in your FOR statement. Why? You should be consistent. Either use the literal everywhere, or better yet, define a variable early on containing the full path to your temp file, and then use that variable consistently when referencing the temp file.

6) You create an excess of variable names. For example, you define TIMESTAMP, only to transfer the value to TIMENOW (or timeXmlCreated), and you never use the value of TIMESTAMP again. You should simply create TIMENOW and timeXmlCreated directly.

7) All of your time / date manipulation is highly dependent on the locale settings of the machine. Your code will break if it is moved to another machine that uses different formats for date and/or time. You should strive for code that is not dependent on locale settings. WMIC DATAFILE, and WMIC OS can be used to get locale indpendent timestamp information.

WMIC timestamps have the format of YYYYMMDDhhmmss.ffffffSzzz

where:

YYYY = gregorian year
MM = month
DD = day
hh = hour in 24 hour format
mm = minutes
ss = seconds
ffffff = fractional seconds (microseconds)
S = timezone sign: + or -
zzz = timezone: minutes difference from UTC


You can use the following to get the current timestamp:

set "localTime="
for /f "skip=1" %%A in (
'wmic os get localdatetime'
) do if not defined localTime set "localTime=%%A"


You can use the following to get the last modified timestamp of a file. Note that WMIC requires that backslashes be escaped as \\ within the path.

set "fileTime="
for /f "skip=1" %%A in (
'wmic datafile where "name='c:\\fullPath\\yourFile.ext'" get lastModified'
) do if not defined fileTime set "fileTime=%%A"


8) Ideally, you want to convert the date and time (timestamp) into an integral number so that you can do simple interval arithmetic. One common format is something called unix time - the number of seconds since midnight Coordinated Universal Time (UTC), January 1, 1970 (disregarding leap seconds). Timestamps before January 1, 1970 have negative unix times. I have written a batch function called :UnixTime to compute the unix time for a given timestamp, or for the current time. You can then simply subtract the file unix time from the current unix time, and look for a value > 900 (15 min * 60 sec/min)

Some important notes about the :UnixTime code.

  • SET /A can access the value of variables directly without expansion.



  • SET /A can make multiple computations and assignments with one call.



  • Batch treats numbers prefixed with 0 as octal. So a value of 09 would be an error, since 9 is not an octal digit. That is why the formula inserts prefixes before the substrings, and then uses modulo arithmetic to derive the correct value.



9) Your code to convert the hour into zero padded 24 hour format is flawed. It gives a value of 12 for 12 AM when it should be 00. It is also needlessly wordy.

Here is one concise way to do it:

if %H_NUM% equ 12 set "H_NUM=00"
if %AM_PM% equ PM set /a H_NUM=1%H_NUM%+12-100
if %H_NUM% lss 10 set "H_NUM=0%H_NUM%"


And here is an even more concise way:

set /a "AM=0, PM=12, H_NUM=(1%H_NUM%-100)%%12+%AM_PM%"
if %H_NUM% lss 10 set "H_NUM=0%H_NUM%"


Here is how I would write the code:

`@echo off
setlocal

set "file=my.xml"
set "execTime=15"

:: get full path to file
for %%F in ("%file%") do set "file=%%~fF"

:: escape the backslashes
set "file=%file:\=\\%"

:: get the last modified timestamp for the file
set "fileTime="
for /f "skip=1" %%A in (
'wmic datafile where "name='%file%'" get lastModified'
) do if not defined fileTime set "fileTime=%%A"

:: convert file timestamp to unix time format
call :UnixTime fileTime %fileTime%

:: get current timestamp in unix time

Context

StackExchange Code Review Q#68336, answer score: 3

Revisions (0)

No revisions yet.