A little help with this code please...

  1. 8 months ago
    Edited 8 months ago

    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

    This is for illustrations purposes only. Do not cut/paste this code without insuring that it has been properly tested to meet you requirements

  2. Dave S

    19 Mar 2017 Answer San Diego, California USA
    Edited 8 months ago

    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

    This is for illustrations purposes only. Do not cut/paste this code without insuring that it has been properly tested to meet you requirements

  3. Thanks Dave,

    Works great.

    Thanks again.

    Lennox

  4. Thomas T

    19 Mar 2017 Pre-Release Testers, Xojo Pro Europe (Germany, Munich)
    Edited 8 months ago

    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.

  5. Thanks Thomas.

    Lennox

  6. Maurizio R

    20 Mar 2017 Pre-Release Testers, Xojo Pro

    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
  7. Tim P

    20 Mar 2017 Pre-Release Testers, Xojo Pro ⭐️

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

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

  8. Maurizio R

    20 Mar 2017 Pre-Release Testers, Xojo Pro

    So the change IS better...

or Sign Up to reply!