List Element Not Found [SOLVED]

[jayand]

I have drafted a simple, if poorly coded, program to read the file names of a folder of e-books.

The file names are something like this:
Book Title (1) (epub),epub
Book Title (2) (azw3),azw3
Book Title (4) (retail) (azw3),azw3
The purpose of the program is to rename the files minus the bracketed (epub), (azw3),(retail) (azw3), etc.
So far everything works perfectly, except that it does not return the string position of (retail) when using FindString.

Code: Select all

Global NewList Directories.s()

Global NewList Name.s()

Global NewList ExtnList.s() ; The actual list is longer than shown here
AddElement(ExtnList())
ExtnList() = " (retail)" ; <- This is the missed lement.
AddElement(ExtnList())
ExtnList() = " (epub)"
AddElement(ExtnList())
ExtnList() = " (azw3)"
AddElement(ExtnList())
ExtnList() = " (mobi)"
AddElement(ExtnList())
ExtnList() = " (html)"
AddElement(ExtnList())
ExtnList() = " (pdf)"

; Possible that not all the following varibals will be used in final code
Temp.s
Fpath.s = "E:\Test\Books\"
FileNew.s
FileOld.s
Fextn.i
Counter.i = 0
FpLen.i = Len(Fpath)
ExtLen.i

Procedure ListFilesRecursive(Dir.s, List Files.s())
  ; Procedure code thanks to Trond https://tinyurl.com/26774zez
  If Right(Dir, 1) <> "\"
    Dir + "\"
  EndIf
  D = ExamineDirectory(#PB_Any, Dir, "")
  While NextDirectoryEntry(D)
    Select DirectoryEntryType(D)
      Case #PB_DirectoryEntry_File
        AddElement(Files())
        Files() = Dir + DirectoryEntryName(D)
      Case #PB_DirectoryEntry_Directory
        Select DirectoryEntryName(D)
          Case ".", ".."
            Continue
          Default
            AddElement(Directories())
            Directories() = Dir + DirectoryEntryName(D)
        EndSelect
    EndSelect
  Wend
  FinishDirectory(D)
  ForEach Directories()
    ListFilesRecursive(Directories(), Files())
  Next
EndProcedure

ListFilesRecursive(Fpath, Name())

ForEach Name()
  Counter + 1
  ForEach ExtnList()
    Temp = Right(Name(), Len(Name()) - FpLen)
    ; Problem with the following
    x = FindString(Temp, ExtnList()) ; This works for everything except the first list element.
    If x > 0
      FileNew = Left(Temp, x - 1) + "*"
      Debug FileNew
    EndIf
  Next
  ; File rename will go here
Next

Debug "File Count = " + Str(Counter)

FreeList(Directories())
FreeList(Name())
FreeList (ExtnList())

Any thoughts or suggestions about what I am doing wrong would be welcome.

Jay

[jassing]

Since directories is "global" you're always changing it; if you adda "debug dir" to the top of your ListFilesRecursive() procedure, you'll see you're always examining the same folder.
You want the list to be local (protected) so you're not adding these list.

[jayand]

Thanks for taking the time to reply, but at this testing stage this is the only folder the code needs to use, no recursion. As i said the code works perfectly except it does not correctly detect the first element of the ExtnList.() list when using FindString otherwise the code works perfectly.

[jassing]

Using a folder with no sub-folders, runs thru to the end w/o issue.

[jayand]

The issue is not reading ALL the files in the folder, every one of the file names are correctly added to the list. Upon reflection your previous post indicated where I could improve the coding style, but I see no reason why it would resolve the problem I currently have.

[infratec]

With subdirectories your recusrive stuff fails.

With a single directory it works as it should:

(retail)
(epub)
(azw3)
(mobi)
(html)
(pdf)

(retail)
(epub)
(azw3)
(mobi)
(html)
(pdf)

(retail)
(epub)
(azw3)
(mobi)
(html)
(pdf)

(retail)
(epub)
(azw3)
(mobi)
(html)
(pdf)

File Count = 4

So (retail) is not missing.

For optimization I put the line:

Code: Select all

Temp = Right(Name(), Len(Name()) - FpLen)

Out of the inner loop, directly below Counter + 1

[jayand]

Thanks for taking the time to reply infratec, but as I said there is only a single directory so recusion is not the issue. The problem is at the line

x = FindString(Temp, ExtnList()) ;

, which works for every other element in ExtnList() except the first (retail) element. I will create another reduced size edition of the code to read from a text file of the name of the files in the directory, without Trond's procedure, which I only used as a quick and dirty option because I didn't have anything more suitable to hand at the time I will report back here with the results and if the problem still exists I will post the new code with a link to the text file. Thanks for the optimization tip.

[Demivec]

I have drafted a simple, if poorly coded, program to read the file names of a folder of e-books.

The file names are something like this:
Book Title (1) (epub),epub
Book Title (2) (azw3),azw3
Book Title (4) (retail) (azw3),azw3
The purpose of the program is to rename the files minus the bracketed (epub), (azw3),(retail) (azw3), etc.
So far everything works perfectly, except that it does not return the string position of (retail) when using FindString.

----code snipped----

Any thoughts or suggestions about what I am doing wrong would be welcome.

I tested your code with the three example filenames listed and findstring worked correctly each time and reported the correct location of ' (retail)'.

Your search loop does not account for more than one search string match and so the results of the latest string search for replaces the previous one(s).

The program's debug output looks like this:

Code: Select all

Book Title (1)*
Book Title (2)*
Book Title (4)*
Book Title (4) (retail)*
File Count = 3

A portion of the code that does not function to achieve the desired outcome:

Code: Select all

If x > 0
  FileNew = Left(Temp, x - 1) + "*"
  Debug FileNew
EndIf

Notice the match for ' (retail)' generates results in FileNew string shown as the third line of the debug output.
Notice the match for ' (azw3)' generates results in FileNew string shown as the fourth line of the debug output, for the same file.

The error is in your code if you are looking for only the earliest match.
In addition to suggestions made by infratec I suggest to modify your loop to something like the following if you only want the earliest match:

Code: Select all

ForEach Name()
  Counter + 1
  Temp = Right(Name(), Len(Name()) - FpLen)
  FileNew = Temp
  
  ForEach ExtnList()
    ; Problem with the following
    x = FindString(Temp, ExtnList()) ; This works for everything except the first list element.
    If x > 0 and x < Len(Temp)
      FileNew = Left(Temp, x - 1) + "*"
      Debug FileNew
    EndIf
  Next
  
  If FileNew <> Temp
    ; File rename will go here
  EndIf
Next

[jayand]

Thanks for taking the time to reply Demivec, but unfortunately you have used the simple illustration of file names shown in the body of my original post, however the real file names are more complex than that.

As per my last reply to infratec I created a simplified edition of the code, minus the ListFilesRecursive procedure which led to a number of side issues being raised in this thread, In the process of creating this simplified version I discovered my, now glaringly obvious mistake . The x variable in the ForEach ExtnList() loop was being changed as it passed through the reaming tests in the loop. All it required was a simple Break when x was set >0, plus I realized the order of the elements in the ExtnList() list was important.

If anyone is interested the following code is provided as an illustration of my solution, not for a critique of my poor coding style. The associated "List_Test.txt" file can be downloaded from: https://www.mediafire.com/file/jcitby7o ... t.txt/file (2KB ) also see comments in the code below.

Code: Select all

Global NewList Books.s()

Global NewList ExtnList.s() ; The order of the elements in this list are important
AddElement(ExtnList())
ExtnList() = " (UK)"
AddElement(ExtnList())
ExtnList() = " (US)"
AddElement(ExtnList())
ExtnList() = " [MM]"
AddElement(ExtnList())
ExtnList() = " (age"
AddElement(ExtnList())
ExtnList() = " (retail)"
AddElement(ExtnList())
ExtnList() = " (epub)"
AddElement(ExtnList())
ExtnList() = " (azw3)"
AddElement(ExtnList())
ExtnList() = " (mobi)"

Fpath.s = "E:\Test\Books\" ; This line simulates reading actual file names from folder
FpLen.i = Len(Fpath)
Temp.s
Counter.i = 0
x.i  ; This was the variable being incorrectly overwritten in the For Each loop

If ReadFile(0,  "E:\Test\List_Test.txt")  ; Change this to actual location of the text file
  While Eof(0) = 0  ; Add file names to Books() list
    AddElement(Books())
    Books() = ReadString(0)
  Wend
  CloseFile(0)
  ;
  ; Now strip the meaningless parts of the file name
  ForEach Books()
    Counter + 1
    ForEach ExtnList()
      x = 0  ; Clear the test variable
      Temp = Right(Books(), Len(Books()) - FpLen)  : Remove the path part
      x = FindString(Temp, ExtnList())  ; Test if element found
      If x >0  ; If element is found
        Break  ; Exit the inner loop, leaving the test x variable at last set value
      EndIf
    Next

    If x > 0  ; Now the Temp string contains the cleaned part of the file name
      Temp = RTrim(Temp)   ;  Strip of the trailing space
      Debug Temp  ; Show the cleaned file names
    EndIf
  Next
  Debug "File Count = " + Str(Counter)  ; Using the test file this will = 28
Else
  MessageRequester("Information","Couldn't open the file!")
EndIf

FreeList(Books())
FreeList(ExtnList())

[Demivec]

Thanks for taking the time to reply Demivec, but unfortunately you have used the simple illustration of file names shown in the body of my original post, however the real file names are more complex than that.

As per my last reply to infratec I created a simplified edition of the code, minus the ListFilesRecursive procedure which led to a number of side issues being raised in this thread, In the process of creating this simplified version I discovered my, now glaringly obvious mistake . The x variable in the ForEach ExtnList() loop was being changed as it passed through the reaming tests in the loop. All it required was a simple Break when x was set >0, plus I realized the order of the elements in the ExtnList() list was important.

I used the only list of file names you supplied at the time I replied. The complexity of the file names did not actually increase in your last solution, only the variety did. The solution I suggested would still have produced the same results as your own final solution if you trimmed off the final ' *' portion of the string. My solution was neither benefited nor harmed by the order of elements in the ExtnList() list.

I'm glad you were able to find a way to zero in on the parts of your code that were failing.