if文の効率的な書き方をご教示いただきたい。

ご教示いただきたいこと

下記if文を書いたのですが、より効率的に記載する方法があればご教示いただきたい。
(同じような処理、エラーメッセージを何度も書いているため、自分自身あまり効率的ではないと感じています。ただどのような書き方が効率的かのノウハウがなく、質問させていただきました。)

コード

def CheckTargetExists(file_path, dst_dir, dst_bucket):
    # 転送対象ファイルの存在確認
    if os.path.isfile(file_path):
        logInfo('転送対象ファイル: {0}が存在します。処理を継続します。'.format(file_path))
        # 一時退避フォルダの存在確認
        if os.path.isdir(dst_dir):
            logInfo('一時退避先フォルダ: {0}が存在します。処理を継続します。'.format(dst_dir))
            # S3バケットの存在確認
            for i in s3.list_buckets()['Buckets']:
                if i['Name'] == dst_bucket:
                    logInfo('転送先バケット: {0}が存在します。処理を継続します。'.format(dst_bucket))
                    break
                else:
                    pass
            else:
                logErr('転送先バケット: {0}が存在しません。処理を終了します。'.format(dst_bucket))
                sys.exit(1)
        else:
            logErr('一時退避先フォルダ: {0}が存在しません。処理を終了します。'.format(dst_dir))
            sys.exit(1)
    else:
        logErr('転送対象ファイル: {0}が存在しません。処理を終了します。'.format(file_path))
        sys.exit(1)
    logInfo('全てのファイル、フォルダ、バケットが存在するため、処理を継続します。')

コードの補足

Windows Server内の特定ファイルを一時退避フォルダにコピーし、コピーしたファイルをS3に転送するスクリプトを書いています。その中で、特定ファイル、一時退避フォルダ、転送先S3バケットの存在確認をする関数が上記コードになります。
file_path: フルパスが入ります。
dst_dir: フルパスが入ります。
dst_bucket: バケット名が入ります。
logInfo(): Infoレベルでログを出力します。
logErr(): Errorレベルでログを出力します。

Orlofsky

2019/04/22 04:36

if分 → if文

legacsilve

2019/04/22 05:26

ご指摘ありがとうございます！修正しました！

行動規範の内容に同意します

回答2件

sys.exitするのであればあえてネストしなくても書けます（return等にしても同じ）。この方が可読性が上がります。

python
1def CheckTargetExists(file_path, dst_dir, dst_bucket):
2    # 転送対象ファイルの存在確認
3    if os.path.isfile(file_path):
4        logInfo('転送対象ファイル: {0}が存在します。処理を継続します。'.format(file_path))
5    else:
6        logErr('転送対象ファイル: {0}が存在しません。処理を終了します。'.format(file_path))
7        sys.exit(1)
8
9    # 一時退避フォルダの存在確認
10    if os.path.isdir(dst_dir):
11        logInfo('一時退避先フォルダ: {0}が存在します。処理を継続します。'.format(dst_dir))
12    else:
13        logErr('一時退避先フォルダ: {0}が存在しません。処理を終了します。'.format(dst_dir))
14        sys.exit(1)
15
16    # S3バケットの存在確認
17    for i in s3.list_buckets()['Buckets']:
18        if i['Name'] == dst_bucket:
19            logInfo('転送先バケット: {0}が存在します。処理を継続します。'.format(dst_bucket))
20            break
21    else:
22        logErr('転送先バケット: {0}が存在しません。処理を終了します。'.format(dst_bucket))
23        sys.exit(1)
24
25    logInfo('全てのファイル、フォルダ、バケットが存在するため、処理を継続します。')
26

6つあるメッセージは定型文で、「存在する対象」「継続するか終了するか」が変わるだけなので、生成する関数を書くことも考えられます。しかし、そうするとコードの見通しがかえって悪くなるので、これはこのままでもいいと思います。

投稿2019/04/22 04:46

hayataka2049

総合スコア30933

legacsilve

2019/04/22 05:27

ご回答いただきありがとうございます。確かに、ネストしないほうがシンプルですね。定型文を関数にするか、という点についてもコメントいただき、大変参考になりました！

行動規範の内容に同意します

ベストアンサー

正常/異常の判定の if 文が沢山入れ子になってしまった場合は、異常の場合は関数を return またはアプリケーションを exit() すれば、入れ子を減らせます。

簡潔なコードの書き方を学びたい場合は以下の書籍がおすすめです。
リーダブルコード

python
1def CheckTargetExists(file_path, dst_dir, dst_bucket):
2    if not os.path.isfile(file_path):
3        logErr('転送対象ファイル: {0}が存在しません。処理を終了します。'.format(file_path))
4        sys.exit(1)
5    logInfo('転送対象ファイル: {0}が存在します。処理を継続します。'.format(file_path))
6
7    if not os.path.isdir(dst_dir):
8        logErr('一時退避先フォルダ: {0}が存在しません。処理を終了します。'.format(dst_dir))
9        sys.exit(1)
10    logInfo('一時退避先フォルダ: {0}が存在します。処理を継続します。'.format(dst_dir))
11
12    # S3バケットの存在確認
13    for i in s3.list_buckets()['Buckets']:
14        if i['Name'] == dst_bucket:
15            logInfo('転送先バケット: {0}が存在します。処理を継続します。'.format(dst_bucket))
16            break
17        else:
18            pass
19    else:
20        logErr('転送先バケット: {0}が存在しません。処理を終了します。'.format(dst_bucket))
21        sys.exit(1)
22    logInfo('全てのファイル、フォルダ、バケットが存在するため、処理を継続します。')