3
\$\begingroup\$

I need to check a condition, and based on the condition perform a very similar for-loop.

import os

folder = r'path'
folder_list = os.listdir(folder)
folder2 = r'path/folder2'
args = True

the_list = []

if args is True:

    for x in folder_list:

        if x.endswith('.txt'):

            if not os.path.exists(os.path.join(folder2, x)):
                full_path = os.path.join(folder, x)
                the_list.append(full_path)
else:

    for x in folder_list:

        if x.endswith('.txt'):
                full_path = os.path.join(folder, x)
                the_list.append(x)

I would like to avoid having the redundant for-loop code if possible. I originally had the following, which I wasn't happy with because args was being checked every single iteration, which is unnecessary.

import os

folder = r'path'
folder_list = os.listdir(folder)
folder2 = r'path/folder2'
args = True
the_list = []


for x in folder_list:

    if x.endswith('.txt'):
        full_path = os.path.join(folder, x)
        if args['i'] is True:

            if not os.path.exists(os.path.join(folder2, x)):
                the_list.append(full_path)

        else:
            the_list.append(full_path)
\$\endgroup\$
7
  • 1
    \$\begingroup\$ Why do you even check args if you set it yourself to True? \$\endgroup\$ – mkrieger1 Jun 22 '17 at 12:40
  • 2
    \$\begingroup\$ @mkrieger1 Sorry, in my actual code that condition changes based on user input. I just simplified it here. \$\endgroup\$ – fpolig01 Jun 22 '17 at 12:41
  • 1
    \$\begingroup\$ And what is 'i'? Please show your actual code, the code you've posted here doesn't actually run. \$\endgroup\$ – mkrieger1 Jun 22 '17 at 12:41
  • 2
    \$\begingroup\$ Also the two versions aren't equivalent. In the first version, in the else part you append x to the_list, while in the second version you append full_path. \$\endgroup\$ – mkrieger1 Jun 22 '17 at 12:44
  • 1
    \$\begingroup\$ @Baldrickk I don't refer to the difference between the if and the else part, but to the difference between the first and second version of the code shown. \$\endgroup\$ – mkrieger1 Jun 22 '17 at 12:45
6
\$\begingroup\$

That's where generators can be very helpful, for example you could use:

if args:
    folderlist = (f for f in folderlist if f.endswith('.txt') and not os.path.exists(os.path.join(folder2, f))
else:
    folderlist = (f for f in folderlist if f.endswith('.txt'))

for x in folderlist:
    full_path = os.path.join(folder, x)
    the_list.append(full_path)

The checks are now done in the "generator expression"s.

You could simplify this some more because the if x.endswith('.txt') is done in both branches:

folderlist = (f for f in folderlist if f.endswith('.txt'))
if args:
    folderlist = (f for f in folderlist if not os.path.exists(os.path.join(folder2, f))

for x in folderlist:
    full_path = os.path.join(folder, x)
    the_list.append(full_path)
\$\endgroup\$
4
  • \$\begingroup\$ Awesome this works! Thanks. If you just say if args: will that be true if it equals True? So I don't need to explicitly say if args is True? \$\endgroup\$ – fpolig01 Jun 22 '17 at 14:05
  • \$\begingroup\$ Would you mind explaining how the generators are working in this context? I've never used them before. \$\endgroup\$ – fpolig01 Jun 22 '17 at 15:24
  • \$\begingroup\$ @fpolig01 The if args just checks for "truthiness". You need to check if it makes sense to do if args or if args is True, you didn't provide enough details to answer this definitively. \$\endgroup\$ – MSeifert Jun 22 '17 at 16:05
  • \$\begingroup\$ @fpolig01 Generators are lazy-evaluating iterators. You could have a look at PEP 289. That contains the rationale for generator expressions and also how they work. \$\endgroup\$ – MSeifert Jun 22 '17 at 16:07
2
\$\begingroup\$

Since, irrespective of if and else loop, for loop is being used then, instead of iterating it in both, it can be taken outside. Similarly, if x.endswith('.txt') is common in both as well so, it can be checked before if and else loop as well. You can try the following:

for x in folder_list:
    if x.endswith('.txt'):
        if args:
            if not os.path.exists(os.path.join(folder2, x)):
                full_path = os.path.join(folder, x)
                the_list.append(full_path)

        else:
            full_path = os.path.join(folder, x)
            the_list.append(full_path)

You can also try using list comprehension may be something like below :

the_list = [os.path.join(folder, x) if (args and not os.path.exists(os.path.join(folder2, x)))
                                    else os.path.join(folder, x) 
                                    for x in folder_list if x.endswith('.txt')]
\$\endgroup\$
0
0
\$\begingroup\$

Put the unique code from each conditional block into different functions, and set the function to call within your loop based on that condition.

def buildlist1(x):
    if not os.path.exists(os.path.join(folder2, x)):
        full_path = os.path.join(folder, x)
        the_list.append(full_path)

def buildlist2(x):
    full_path = os.path.join(folder, x)
    the_list.append(x)

buildlist = buildlist1 if args else buildlist2

for x in folder_list:
    if x.endswith('.txt'):
        buildlist(x)
\$\endgroup\$
0
\$\begingroup\$

You did identify the problem - violating the rule "do not repeat yourself". for your problem you should think about the "data flow", not so much about "control structures". if you process data in "stages" you are more likely to get a modular and testable program.

# get full list
folder_list = os.listdir(folder)

# filter for textfiles
folder_list = [x for x in folder_list if x.endswith('.txt')]

# conditionally filter for new files
if args:
    folder_list = [x for x in folder_list if not os.path.exists(os.path.join(folder2, x))]

# final file list
the_list = [os.path.join(folder, x) for x in folderlist]
\$\endgroup\$

Your Answer

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