A little help with this code please...

Hello.

I have a folder in “Pictures” folder named “My Images”, it currently has 3 images in it.

I am using code such that if there are images in “My Images” folder then the code will create a folder in “My Images” folder then move the existing images to that folder.
I am aware that the folder count will now be incremented by 1.

  
  Dim i as integer
  Dim MyImagesFolderFolderitems as FolderItem
  Dim ExistingImagesInMyImagesFolder, FolderForExistingImagesInMyImagesFolder as FolderItem
  
  
  For i = MyImagesFolder.Count downto 1
    MyImagesFolderFolderitems = MyImagesFolder.TrueItem(i)
    
    If MyImagesFolderFolderitems.name.Right(4) = ".jpg" or _
      MyImagesFolderFolderitems.name.Right(5) = ".JPEG" or _
      MyImagesFolderFolderitems.name.Right(4) = ".bmp" or _
      MyImagesFolderFolderitems.name.Right(4) = ".png" or _
      MyImagesFolderFolderitems.name.Right(4) = ".tif" then
      ThereAreExistingImagesInMyImagesFolder = True
      
      Exit
      
    end if
  next i
  
  'app.gComposeSQLSavingDateTime is a method in app...
  'Dim d as new date
  'Return ReplaceAll(d.SQLDatetime, ":", "")
  
  
  FolderForExistingImagesInMyImagesFolder = MyImagesFolder.Child("Previously Existing Images " + app.gComposeSQLSavingDateTime + " folder")
  // this will change My Images.Count by +1, 
  // when the if-endif loop below starts the OS would not have updated My Images as yet and a NOE will be thrown
  If FolderForExistingImagesInMyImagesFolder <> nil and not FolderForExistingImagesInMyImagesFolder.Exists then FolderForExistingImagesInMyImagesFolder.CreateAsFolder
  
// Apparently,  the OS would not have updated app My Images as yet and a NOE will be thrown
  If ThereAreExistingImagesInMyImagesFolder = True Then
    For i = MyImagesFolder.Count downto 1
      MyImagesFolderFolderitems = MyImagesFolder.TrueItem(i)
      
      If MyImagesFolderFolderitems.name.Right(4) = ".jpg" or _  // NOE occurs here
        MyImagesFolderFolderitems.name.Right(5) = ".JPEG" or _
        MyImagesFolderFolderitems.name.Right(4) = ".bmp" or _
        MyImagesFolderFolderitems.name.Right(4) = ".png" or _
        MyImagesFolderFolderitems.name.Right(4) = ".tif" then
        
        MyImagesFolderFolderitems.MoveFileTo FolderForExistingImagesInMyImagesFolder
        
      end if
    next
    
  End if
  

How can I fix that?

Thanks.

Lennox

instead of looping thru the folder and checking extensions TWICE… do it one, and collect a list of the files.
if that list.ubound>=0 then you have some images (instead of using hereAreExistingImagesInMyImagesFolder)

		    Dim i As Integer
    Dim listoffiles() As FolderItem
    Dim MyImagesFolderFolderitems As FolderItem
    Dim ExistingImagesInMyImagesFolder, FolderForExistingImagesInMyImagesFolder As FolderItem
    
    For i = MyImagesFolder.Count DownTo 1
        MyImagesFolderFolderitems = MyImagesFolder.TrueItem(i)
        If MyImagesFolderFolderitems.name.Right(4) = ".jpg" Or _
            MyImagesFolderFolderitems.name.Right(5) = ".JPEG" Or _
            MyImagesFolderFolderitems.name.Right(4) = ".bmp" Or _
            MyImagesFolderFolderitems.name.Right(4) = ".png" Or _
            MyImagesFolderFolderitems.name.Right(4) = ".tif" Then
            listoffiles.append(MyImagesFolderFolderitems)   
        End If
    Next i
    
    'app.gComposeSQLSavingDateTime is a method in app...
    'Dim d as new date
    'Return ReplaceAll(d.SQLDatetime, ":", "")
    
    
    FolderForExistingImagesInMyImagesFolder = MyImagesFolder.Child("Previously Existing Images " + app.gComposeSQLSavingDateTime + " folder")
    // this will change My Images.Count by +1, 
    // when the if-endif loop below starts the OS would not have updated My Images as yet and a NOE will be thrown
    If FolderForExistingImagesInMyImagesFolder <> Nil And Not FolderForExistingImagesInMyImagesFolder.Exists Then FolderForExistingImagesInMyImagesFolder.CreateAsFolder
    
    // Apparently,  the OS would not have updated app My Images as yet and a NOE will be thrown
    If listoffiles.ubound>=0 Then
        For i = 0 to listoffiles.Ubound   
            listoffiles(i).MoveFileTo FolderForExistingImagesInMyImagesFolder
        Next
    End If

Thanks Dave,

Works great.

Thanks again.

Lennox

It should be well known by now that iterating over FolderItem contents in reverse is very inefficient, and should not be propagated.

If the folder only contains 10 files, it’ll be of no issue, but if it’s 1000 files, it’ll get VERY slow, it can end up taking minutes, at worst.

So, the correct way is this, as it’s also shown in the docs:

Iterate forward, and collect all FolderItems into an array.

Then walk over the array. At that point, you can create or delete files in the folder, without it affecting your previously collected array of folderitems. – Which is already done anyway.

So, simply change

For i = MyImagesFolder.Count DownTo 1

into

For i = 1 to MyImagesFolder.Count

Also, it would be smart to add this line before the if line, because items can be nil if they’re not accessible for some reason (permissions or bad links, for instance):

if MyImagesFolderFolderitems = nil then continue

And ideally you’d not call MyImagesFolderFolderitems.Name repeatedly as that’s relatively slow as well. Better assign it once to a String and then use that in the many if terms.

Thanks Thomas.

Lennox

I have noticed that folderitem.count is very slow (at least on Windows) so is better to write:

dim count As Integer = MyImagesFolder.Count for i = 1 to count

It’s a slow process, as noted in the docs :slight_smile:
http://documentation.xojo.com/index.php/FolderItem.Count

So the change IS better…