How can I make my batch script for deletion of old files work?

Question

Inside a folder (say c:\test) I want to delete the oldest file if the number of files are over 21.

This is what I came up with, issue is that it one time deletes the oldest file, second run it does nothing and deletes the oldest for third, continues like that. Also it doesn't care if the file amount is lower than 21, deletes even if it is lower.

Here is the code:

SET BACKUPDIR=C:\test
SET countfiles=dir BACKUPDIR /b | find /v /c "::"

if countfiles GTR 21 (
FOR /F %%i IN ('DIR /B /O-D %BACKUPDIR%') DO SET OLDEST=%%i
DEL %BACKUPDIR%\%OLDEST%
)

Answer 1

There are a number of problems with this.

1. Environment variables vs. literal strings:

   SET countfiles=dir BACKUPDIR /b | find /v /c "::"
   

   Here you're trying to list files in a directory named "BACKUPDIR", instead of using the environment variable. It should be

   SET countfiles=dir %BACKUPDIR% /b | find /v /c "::"
   

2. Your try of getting the number of files:

   SET countfiles=dir %BACKUPDIR% /b | find /v /c "::"
   

   can't possibly work. This simply assigns the string 'dir BACKUPDIR /b | find /v /c "::" ' to the environment variable countfiles. It won't run anything—how should cmd know that this is a command to execute?

   What you want is the following:

   for /f %%x in ('dir %BACKUPDIR% /b ^| find /v /c "::"') do set countfiles=%%x
   

   This will set the contents of the countfiles environment variable to the output of above command. You can find more information on that with help for.

3. Your comparison:

   if countfiles GTR 21
   

   this compares the string "countfiles" and the string "21". And since countfiles gets sorted after 21, this condition will always be true.

   You want to use the environment variable %countfiles% here:

   if %countfiles% GTR 21
   

4. You try "normal" variable expansion inside a "block":

   if %countfiles% GTR 21 (
       FOR /F %%i IN ('DIR /B /O-D %BACKUPDIR%') DO SET OLDEST=%%i
       DEL %BACKUPDIR%\%OLDEST%
   )
   

   Since cmd expands environment variables immediately when parsing a command this leads to problems. Note that a block like above counts as a single command for cmd. The complete block will be parsed be fore it's run and all variables inside are replaced already when it runs, so that's not the right way to deal with variables that might change inside that block.

   The solution here is delayed expansion. You can include the following line at the start of your batch:

   setlocal enabledelayedexpansion
   

   and instead of using the variable like %OLDEST% you use !OLDEST!. The ! expand variables too, but do so directly before execution instead of while parsing. So your snippet would become:

   if %countfiles% GTR 21 (
       FOR /F %%i IN ('DIR /B /O-D %BACKUPDIR%') DO SET OLDEST=%%i
       DEL %BACKUPDIR%\!OLDEST!
   )
   

5. Just in case you are expecting file names with spaces, the for /f loop in the last block doesn't work as expected. You can modify it like this:

   for /f "delims=" %%i in ...
   

   This ensures that the line isn't broken at spaces (you would have got just the first token–up to the first space in the file name–instead of the complete file name.

So, summarizing, your program could look like the following:

SET BACKUPDIR=C:\test
for /f %%x in ('dir %BACKUPDIR% /b ^| find /v /c "::"') do set countfiles=%%x

if %countfiles% GTR 21 (
    FOR /F "delims=" %%i IN ('DIR /B /O-D %BACKUPDIR%') DO SET OLDEST=%%i
    DEL %BACKUPDIR%\!OLDEST!
)

and should roughly work. I haven't tested every aspect of it (bed calls) but I'm confident it works way better than before and I hope you understand above fixes and why they are necessary.

By the way: find /v /c "::" is a nifty little trick. Didn't know that one or thought of it.

Just in case: If you actually wanted to continue deleting the oldest file until your file count drops below 21, then that single if won't cut it. You need to set up a loop, either by continuing to iterate and re-count every iteration or by just calculating how many files you have to delete. I'll leave this as an exercise, though. (Hint: Loops can be done with labels and conditional gotos or by using for /l. The latter one will require more delayed expansion, though, so be careful.)

Answer 2

here's an alternative in vbscript, that iterates the directory just once.

Set objFS=CreateObject("Scripting.FileSystemObject")
Set objArgs = WScript.Arguments
strFolder = objArgs(0)
strScriptName = WScript.ScriptFullName 'get script name
Set objFolder = objFS.GetFolder(strFolder)
Set dict = CreateObject("Scripting.Dictionary") 
current  = Now
temp=0
count=0 'count files
For Each strFile In objFolder.Files
    If strFile.Path <> strScriptName Then 'do if not script name
      count=count+1
      dict.Add strFile.Path, strFile.DateLastModified
      'if there are more than 21 files
      If count >= 21 Then
      flag=1
      End If
    End If
Next

filedate = dict.Items
filename = dict.keys
If flag = 1 Then
    For i = 0 To dict.Count -1 'iterate dictionary
      diffdate = DateDiff("d",filedate(i),now)
      If num_days_diff >= temp Then
        oldestfile = filename(i)
        oldestdate = filedate(i)
        temp = num_days_diff
      End If             
    Next
End If 
Set dict=Nothing
WScript.Echo "Oldest file: " & oldestfile, oldestdate
objFS.DeleteFile(oldestfile)

save as deletefile.vbs and on command line

c:\test> cscript /nologo deletefile.vbs c:\path\somewhere