2016-02-17 1 views
0

Je travaille sur un script pour un flux de travail automatisé. Il s'attend à ce qu'un fichier CSV et une image disque soient présents dans le répertoire fourni sous args.input.Comment rendre cette fonction plus efficace sur Python?

Je veux vérifier et gérer tous les scénarios possibles: pas de fichier CSV, pas d'image disque, trop de fichiers CSV, trop d'images disque et toute combinaison possible. Je l'ai écrit ci-dessous, qui fonctionne, et semble être lisible par l'homme, mais il semble juste incroyablement excessif et verbeux - est-il possible de le rendre plus compact, tout en conservant sa lisibilité?

# CONFORMANCE CHECKS 
def check_conformance(): 
    csv = glob.glob(args.input+'*.csv') 
    disk_image = glob.glob(args.input+'*.E01') 

    if len(csv) == 1: 
     does_csv_exist = os.path.isfile(csv[0]) 
    elif len(csv) < 1: 
     does_csv_exist = False 
    elif len(csv) > 1: 
     does_csv_exist = "too many CSVs!" 
    if len(disk_image) == 1: 
     does_E01_exist = os.path.isfile(disk_image[0]) 
    elif len(disk_image) < 1: 
     does_E01_exist = False 
    elif len(disk_image) > 1: 
     does_E01_exist = "too many Disk Images!" 
    if len(disk_image) > 1 and len(csv) > 1: 
     does_csv_exist = "too many CSVs!" 
     does_E01_exist = "too many disk images!" 
     return (False, does_csv_exist, does_E01_exist,) 
    if does_E01_exist is True and does_csv_exist is True: 
     return True 
    elif does_E01_exist is True and does_csv_exist is False: 
     return (False, "CSV is missing") 
    elif does_E01_exist is False and does_csv_exist is True: 
     return (False, "E01 disk image is missing") 
    elif does_E01_exist is False and does_csv_exist is False: 
     return (False, "E01 disk image AND csv are missing") 
    elif does_csv_exist is not True and does_csv_exist is not False: 
     return (False, does_csv_exist) 
    elif does_E01_exist is not True and does_E01_exist is not False: 
     return (False, does_E01_exist) 
+1

Cette question pourrait être mieux adaptée à [codereview.se]. Cela ne semble toutefois pas être le cas de Pythonic. – jonrsharpe

+0

n'était pas au courant de ce SE - parfait. Va poster là-bas. – dongle

+0

[Cross-posting] (http://codereview.stackexchange.com/q/120350/9357) sur l'examen du code. –

Répondre

1

Je ne sais pas exactement quel est le but de cette fonction est, mais voici quelques conseils:

  • Une fonction doit avoir une seule fonction. Le vôtre semble avoir plusieurs - retourner si oui ou non l'entrée est conforme à vos normes (True/False) et retourner une sorte de chaîne d'erreur (str). Vous renvoyez des tuples qui combinent ces deux choses de manière imprévisible. Soit choisir l'un ou l'autre, ou normaliser le tuple et toujours retourner la même exacte (à savoir (bool, str))

  • Même si vous pouvez définir plusieurs types différents pour la même variable, vous ne devriez pas. Ne pas définir une valeur booléenne dans une condition, puis une chaîne dans une autre condition (voir does_csv_exist)

je l'aurais fait quelque chose comme ceci:

# CONFORMANCE CHECKS 
# Returns a list of error strings encountered, empty list if OK 
def getConformanceErrors(): 
    csv = glob.glob(args.input+'*.csv') 
    disk_image = glob.glob(args.input+'*.E01') 

    msg = [] 

    if len(csv) < 1: 
     msg.append("CSV is missing") 
    elif len(csv) > 1: 
     msg.append("Too many CSVs") 

    if len(disk_image) < 1: 
     msg.append("Disk Image is missing") 
    elif len(disk_image) > 1: 
     msg.append("Too many Disk Images") 

    return msg