CODEWITHSUNDEEP
vbscript
kgarvey
kgarvey

Reputation: 41

Too many iterations in loop

This script collects all files in a folder and renames the files by appending the number of lines to the file name. All files are .txt files. The method (since fso.MoveFile and fso.DeleteFile are too particular, generating permissions errors) is to

  1. create the text files,
  2. then create a collection of the files in the folder,
  3. then copy each file into the same folder with a new name, and
  4. finally to delete the original file that was copied.

The script works ok, unless there are no empty text files in the collection. What happens is, the collection gets rebuilt with the new files and the script once again renames the files. I know I can prevent this by checking each file for the existence of certain repeating character strings, but I'd like to know what's happening? Why does the script rebuild the file collection and run through them again renaming each one? This continues on until I kill the process.

Another interesting factoid is, if I happen to trap an empty text file, my message is displayed and the script stops there, but has still reprocessed the first file in the collection a second time. Note that the empty file just happens to be the last one in the collection, but the first filed is once again processed.

So, by design a created text file named 'ab0.txt' gets renamed to 'ab0-15.txt' since it has 15 lines of text in it. What happens is this newly renamed file looks like 'ab0-15-15-15-15-15-15-15-15-15-15.txt'

Questions: What's going on? And is there a better and more efficient way to accomplish this objective?

Here's the code pertinent to the issue:

Set fso = CreateObject("Scripting.FileSystemObject")
Set oFolder = fso.GetFolder(strSaveTo)
Set colFiles = oFolder.Files

' Call Sub to copy and rename
ChangeFileName colFiles
MsgBox("File renaming complete.")
' Exit code

Sub ChangeFileName(collectionSet)
  Const ForReading = 1
  Dim oFile

  For Each oFile In collectionSet
    Set LineCnt = fso.OpenTextFile(oFile, ForReading)
    If oFile.Size = 0 then
      'if this msg is not included, weird things happen
      MsgBox("The file named " & oFile & _
        " is empty.You may want to verify and manually delete it.")
      '[I had some code in here to delete the empty file, but nothing worked]
    Else
      Do While LineCnt.AtEndOfStream <> True
        LineCnt.SkipLine
      Loop
      lineVar = lineCnt.Line-1
      strNewFile = strSaveTo & Left(oFile.name, Len(oFile.name)-4) & _
                   "-" & lineVar & ".txt"
      fso.CopyFile oFile, strNewFile
      LineCnt.Close
      fso.DeleteFile oFile, True
    End If
  Next
End Sub

Upvotes: 2

Views: 232

Answers (3)

RLH
RLH

Reputation: 1593

  1. You should create your vars in the scope they are used (e.g. your file/folder objects are used in the sub.
  2. Always explicit(ly) declare your vars.
  3. You don't need to copy the file and rename it then do the delete. Just rename it with the FileObject.Name property.

Here is an example:

Option Explicit 'always declare your vars!

Dim strFolder:    strFolder = "c:\temp\Rename Test"
Dim strExtension: strExtension = "txt"

' Call Sub to rename the files in the folder
ChangeFileName strFolder, strExtension

Sub ChangeFileName(strFolder, strExtension)
    Const ForReading = 1
    Dim FSO:        set FSO = CreateObject("Scripting.FileSystemObject")
    Dim objFolder:  set objFolder = FSO.GetFolder(strFolder)
    Dim colFiles:   set colFiles = objFolder.Files
    Dim objFile
    Dim intCount
    Dim strFileName
    Dim objTextStream

    For Each objFile In colFiles
            msgbox "File: " & objfile.path & vbcrlf & FSO.GetExtensionName(objFile.path)

        if UCase(FSO.GetExtensionName(objFile.Path)) = UCase(strExtension) and _ 
           objFile.Size > 0 then
            'set LineCnt = FSO.OpenTextFile(objFile, ForReading)
            set objTextStream = objFile.OpenAsTextStream(ForReading,-2)
            intCount = 0
            strFileName = objFile.Name
            Do While objTextStream.AtEndOfStream <> True 
                intCount = intCount + 1
                objTextStream.ReadLine
            Loop
            objTextStream.Close
            objFile.Name = FSO.GetBaseName(objFile.Path) & "-" & _
                           intCount & "." & FSO.GetExtensionName(objFile.Path)
        end if
    Next
End Sub

Upvotes: 1

John Coleman
John Coleman

Reputation: 51998

It seems that collectionSet is the collection of files in the folder that you are trying to modify. The problem is that with each pass through the for-each loop you are adding files to this folder, some of which are fed back into the loop. What you need to do is the find a way to take a snapshot of the folder before you try to iterate over it. The way to do this would be to replace the folder collectionSet by a collection of strings which are the names of the files before you iterate over it, and modify your code to open the files by their name (instead of via a file object). That way the collection won't be expanding while you iterate over it.

Upvotes: 1

Cheran Shunmugavel
Cheran Shunmugavel

Reputation: 8459

I've heard anecdotal evidence that the Files collection is "live", meaning that newly created files will be added to the collection and iterated over, but I can't find any documentation that says one way or the other. In any case, it's probably a good idea to copy the File objects in the collection to an array first before processing them:

Dim oFile
Dim fileArray()
Dim i

ReDim fileArray(collectionSet - 1)
i = 0
For Each oFile in collectionSet
    Set fileArray(i) = oFile
    i = i + 1
Next

For Each oFile In fileArray
    ' Count lines and rename
Next

Upvotes: 3

Related Questions