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

Batch file-download script - is it necessary to re-phrase commands?

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

Problem

I run a script to download a file and unzip it. An import process then runs and leaves the files in their source location. I wrote the fully functioning code below to clean up that directory to be ready to use the next night.

I streamlined as much as I believe possible. I realize ~15 lines of code isn't exactly verbose or complicated and I've included the full script for reference. I feel, however, that specifically the second block - line SET DATE= through line DIR - can be better written but I don't know in what way. Am I simply seeing messy code where none exists? It just doesn't seem to 'read' well.

A note - the index.txt file is the same despite having two paths and is located in one or the other depending on the outcome (errors or not) of the import.

SET PATH="\\server\path\path2"
IF NOT EXIST %PATH%\Upload.zip GOTO Done

SET DATE=%date:~4,2%-%date:~7,2%-%date:~10,4%
MKDIR %PATH%\Archives\%date%
SET ARCHIVE=%PATH%\Archives\%date%\
DIR %PATH% /B > %ARCHIVE%log-%date%.txt

MOVE %PATH%\Upload.zip %ARCHIVE%Upload-%date%.zip
MOVE %PATH%\index.txt %ARCHIVE%index-%date%.txt ||(
MOVE %PATH%\error_files\index.txt %ARCHIVE%index-%date%.txt
)
DEL %PATH%\*.tif
RMDIR %PATH%\error_files

PUSHD %PATH%\Archives &&(
FORFILES /S /D -15 /C "CMD /C IF @isdir == TRUE RD /S /Q @path"
) & POPD

:Done

Solution

I don't think there's a better way to format your %DATE% variable. And be careful, because I think this won't work in all locales. I had this problem a few years ago, and I remember that there was no good solution to format dates using native commands. I had to make a simple dateformat utility myself to generate date formats easily and in a locale independent way.

Just a small thing, I recommend NOT to put the trailing \ in the %ARCHIVE% variable. So instead of this:

SET ARCHIVE=%PATH%\Archives\%date%\
DIR %PATH% /B > %ARCHIVE%log-%date%.txt

MOVE %PATH%\Upload.zip %ARCHIVE%Upload-%date%.zip


Write like this:

SET ARCHIVE=%PATH%\Archives\%date%
DIR %PATH% /B > %ARCHIVE%\log-%date%.txt

MOVE %PATH%\Upload.zip %ARCHIVE%\Upload-%date%.zip


This may be a matter of taste, but I like to see explicitly that it's a directory there, not just a prefix.

Another formatting tip about this:

MOVE %PATH%\index.txt %ARCHIVE%\index-%date%.txt ||(
MOVE %PATH%\error_files\index.txt %ARCHIVE%\index-%date%.txt
)


I find it easier to read when the commands inside (...) are indented, like this:

MOVE %PATH%\index.txt %ARCHIVE%\index-%date%.txt || (
    MOVE %PATH%\error_files\index.txt %ARCHIVE%\index-%date%.txt
)

Code Snippets

SET ARCHIVE=%PATH%\Archives\%date%\
DIR %PATH% /B > %ARCHIVE%log-%date%.txt

MOVE %PATH%\Upload.zip %ARCHIVE%Upload-%date%.zip
SET ARCHIVE=%PATH%\Archives\%date%
DIR %PATH% /B > %ARCHIVE%\log-%date%.txt

MOVE %PATH%\Upload.zip %ARCHIVE%\Upload-%date%.zip
MOVE %PATH%\index.txt %ARCHIVE%\index-%date%.txt ||(
MOVE %PATH%\error_files\index.txt %ARCHIVE%\index-%date%.txt
)
MOVE %PATH%\index.txt %ARCHIVE%\index-%date%.txt || (
    MOVE %PATH%\error_files\index.txt %ARCHIVE%\index-%date%.txt
)

Context

StackExchange Code Review Q#61405, answer score: 3

Revisions (0)

No revisions yet.