How to simplify long winded IF / Else statement-CodePudding

Faily new new at Python and self taught by googling and short online courses. I am really enjoying Python and want to start cleaning up some of my old code. I'm looking at a way to simplify my long winded, repeated IF/Else statements. Wondering if there is a better way to do it or it is fine what I am doing, maybe a class or something like that.

Here is a sample code of one of the scenarios I am refering too.

for file in os.listdir(src_dir):
    if raceCheck == 'Race_1':
        if fnmatch.fnmatch(file, '*1.pdf'):
            upload_to_aws_site1(os.path.join(src_dir, file), 'official')
            upload_to_aws_site2(os.path.join(src_dir, file))
            print(file)
    elif raceCheck == 'Race_2':
        if fnmatch.fnmatch(file, '*2.pdf'):
            upload_to_aws_site1(os.path.join(src_dir, file), 'official')
            upload_to_aws_site2(os.path.join(src_dir, file))
            print(file)
    elif raceCheck == 'Race_3':
        if fnmatch.fnmatch(file, '*3.pdf'):
            upload_to_aws_site1(os.path.join(src_dir, file), 'official')
            upload_to_aws_site2(os.path.join(src_dir, file))
            print(file)
    elif raceCheck == 'Race_4':
        if fnmatch.fnmatch(file, '*4.pdf'):
            upload_to_aws_site1(os.path.join(src_dir, file), 'official')
            upload_to_aws_site2(os.path.join(src_dir, file))
            print(file)
    else:
        print('I am here: ', file)

Thank you

CodePudding user response：

The absolute simplest (and arguably most Pythonic) way to handle this is to use a mapping of race to filename, and then check if the race number exists in the mapping, and if so use the filename. As another similar answer suggested, you actually don't need to loop over the dict race mapping object at all.

race_to_filename = {
    'Race_1': '*1.pdf',
    'Race_2': '*2.pdf',
    'Race_3': '*3.pdf',
    'Race_4': '*4.pdf',
}

for file in os.listdir(src_dir):
    if raceCheck in race_to_filename:
        filename = race_to_filename[raceCheck]
        if fnmatch.fnmatch(file, filename):
            upload_to_aws_site1(os.path.join(src_dir, file), 'official')
            upload_to_aws_site2(os.path.join(src_dir, file))
            print(file)
    else:
        print('I am here: ', file)

Also, note that if your race names and filenames are always similar to above, then you can instead define the mapping using a dict comprehension, to save yourself a bit of time:

race_to_filename = {f'Race_{i}': f'*{i}.pdf' for i in range(1, 5)}

CodePudding user response：

Get the last character of raceCheck and check if the filename ends with that character plus .pdf.

Also, since raceCheck doesn't change in the loop, you should do that part outside the loop.

if raceCheck.startsWith('Race_'):
    digit = raceCheck[-1]
    for file in os.listdir(src_dir):
        if file.endswith(digit   '.pdf'):
            upload_to_aws_site1(os.path.join(src_dir, file), 'official')
            upload_to_aws_site2(os.path.join(src_dir, file))
            print(file)
else:
    print("Don't know what to do with", raceCheck)

You could also use glob.glob() instead of your own filename loop.

if raceCheck.startsWith('Race_'):
    digit = raceCheck[-1]
    for file in glob.glob(os.path.join(src_dir, f"*{digit}.pdf")):
        upload_to_aws_site1(file, 'official')
        upload_to_aws_site2(file)
        print(os.path.basename(file))

CodePudding user response：

Not sure what fnmatch does since your example is not reproducible.

Use the symmetry to your advantage, like so:

if raceCheck[-1] == file_name.split(".")[0][-1] and 0 < int(raceCheck[-1]) < 5:
            upload_to_aws_site1(os.path.join(src_dir, file), 'official')
            upload_to_aws_site2(os.path.join(src_dir, file))
            print(file)

This checks whether the last letter in raceCheck matches the last letter in the filename.

You can also concatenate multiple conditions with or or and

CodePudding user response：

The first thing to do is to avoid repeating code that is 100% identical and focus on what varies. Then, if you need a structure that looks like a switch-statement, you can define this switch function:

def switch(v): yield lambda *c: v in v

and use it in a on-iteration for loop:

for file in os.listdir(src_dir):
    for case in switch(raceCheck):
        if   case('Race_1') : mask = '*1.pdf' 
        elif case('Race_2') : mask = '*2.pdf'
        elif case('Race_3') : mask = '*3.pdf'
        elif case('Race_4') : mask = '*4.pdf'
        else: 
            print('I am here: ', file)
            break
        if fnmatch.fnmatch(file, mask):
            upload_to_aws_site1(os.path.join(src_dir, file), 'official')
            upload_to_aws_site2(os.path.join(src_dir, file))
            print(file)
        print('I am here: ', file)

I'm assuming your actual code is more complex than that and warrants a switch-statement structure. Otherwise, a simple dictionary would suffice:

masks = {f'Race_{n}':f'*{n}.pdf' for n in range(1,5)}
for file in os.listdir(src_dir):
    mask = masks.get(raceCheck)
    if not mask: 
        print('I am here: ', file)
        continue
    if fnmatch.fnmatch(file, mask):
        upload_to_aws_site1(os.path.join(src_dir, file), 'official')
        upload_to_aws_site2(os.path.join(src_dir, file))
        print(file)

CodePudding user response：

One of most the common ways (in Python, at least) to refactor long, repetitive if/else branches like this is to use a loop dicts:

mapping = {
    'Race_1': '*1.pdf',
    'Race_2': '*2.pdf',
    'Race_3': '*3.pdf',
    'Race_4': '*4.pdf',
}

for file in os.listdir(src_dir):
    for key, value in mapping.items():
        if raceCheck == key and fnmatch.fnmatch(file, value):
            upload_to_aws_site1(os.path.join(src_dir, file), 'official')
            upload_to_aws_site2(os.path.join(src_dir, file))
            print(file)
            break
    else:
        print('I am here: ', file)