1
\$\begingroup\$

I have files in subfolders on a network drive. I want to create a list of files, and insert that list into an Oracle table.

I can successfully iterate through the files in the subfolders and do the insert:

  • Loop through a user-defined list of subfolders called subfolder_list (I only want the files in specific subfolders; not all of the subfolders)
  • Loop through the files in each subfolder
  • Using an ArcSDESQLExecute class in the ArcPy site package, insert each file name into the records table

import arcpy
import os

db_conn_folder = r'C:\connection\folder\connection.sde'
conn = arcpy.ArcSDESQLExecute(db_conn_folder)

folder = r'\\my\network\folder'
subfolder_list = [r'\DRAWINGS', r'\DOCUMENTS', r'\VIDEOS']

#Delete all records in the table
db_exec = conn.execute('delete from records')

#Loop through the list of subfolders
for i in subfolder_list:
    #loop through the files in the subfolder
    for filename in os.listdir(folder + i):
        #insert the file names into the table
        db_exec = conn.execute('insert into records (file_name) values (' + filename + '\')')

#Commit the insert statements
db_exec = conn.execute('commit')

print "Complete."

The SQL is a little hard to read with all the python parsing and formatting. Plain-language SQL would look like this:

delete from records;
insert into records (file_name) values ('plan_profile_1.tif');
commit;

While the code works just fine, I'm wondering, can it be improved?

(I can count the number of Python scripts I've written on one hand.)

\$\endgroup\$
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Mast Jan 11 '18 at 15:55
  • \$\begingroup\$ @Mast I deleted it. \$\endgroup\$ – Wilson Jan 11 '18 at 16:49
4
\$\begingroup\$
folder = r'\\my\network\folder'
subfolder_list = [r'\DRAWINGS', r'\DOCUMENTS', r'\VIDEOS']

Don't write the type into the variable name. The type should be able to change without the variable having to be renamed. subfolders already tells you that it is a collection of subfolders. subfolder_names might be better, to make clear that it contains the folders' names, not wrapper objects for the file system.

Removing the backslash from the beginning can prevent you from having to read up the code to make sure where the backslash is, at the beginning or the end. subfolder or subfolder_name implies just the name. So add the backslash wherever you concatenate the strings.


#Delete all records in the table
db_exec = conn.execute('delete from records')

That comment is obsolete, as it adds no information that is not already in the next line of code.


#Loop through the list of subfolders
for i in subfolder_list:
    #loop through the files in the subfolder
    for filename in os.listdir(folder + i):
        #insert the file names into the table
        db_exec = conn.execute('insert into records (file_name) values (' + filename + '\')')

Again, the comments are not needed, and instead impair readability. Instead of explaining with comments what the code does, improve the code to better explain itself. i can be acceptable as a loop variable, but whenever possible, use a more explicit name, like in this case subfolder_name. Then you get this more expressive code:

for subfolder_name in subfolder_names:
    for file_name in os.listdir(folder + r'\' + subfolder_name)
        db_exec = conn.execute('insert into records (file_name) values (' + file_name + '\')')

#Commit the insert statements
db_exec = conn.execute('commit')

Again the comment is not needed.


print "Complete."

What is complete? Give the user more info, e. g. `"42 file names inserted into database."

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.