アンチパターンコード 「ウォーターフォール型関数呼び出し(仮)」

ウォーターフォール型関数呼び出し(仮)とは

関数内で何か処理をして別の関数を呼び出し、呼び出し先でもまた何か処理をして別の関数を呼び出す、という深い呼び出し階層になったコードです。

ウォーターフォール型関数呼び出し(仮)

def func1():
    # process 1
    # :
    func2()

def func2():
    # process 2
    # :
    func3()

def func3():
    # process 3
    # :
:

例：テキスト変換スクリプト

例として、テキストファイルを読み込んで大文字を小文字に変換して別のファイルへ出力するスクリプトをウォーターフォール型関数呼び出し(仮)で書いてみます。

例：テキスト変換スクリプト

import sys

def main(input_file, output_file):
    with open(input_file, 'r') as f:
        lines = f.readlines()
    convert_lines(lines, output_file)

def convert_lines(input_lines, output_file):
    output_lines = [line.lower() for line in input_lines]
    write_file(output_file, output_lines)

def write_file(output_file, lines):
    with open(output_file, 'w') as f:
        f.writelines(lines)

if __name__ == '__main__':
    main(sys.argv[1], sys.argv[2])

問題点

• コードが追いにくい
  • 呼び出し階層が深く、全体でやっていることを追うのがとても面倒
• 適切な関数名が付けにくい
  • convert_linesは各行の変換だけでなくファイルの書き込みもおこなっているので、不適切な関数名
  • かといって他の名前も付けにくい
• テストがしにくい
  • convert_linesでは各行の変換だけでなくファイルの書き込みもおこなっているので、これをテストするには書き込んだファイルを検証しなくてはならない

改善

• 各関数でやることを1つにする(他のことをやっている関数を呼び出さない)
• 全体を制御する関数を追加する

改善前

def func1():
    # process 1
    # :
    func2()

def func2():
    # process 2
    # :
    func3()

def func3():
    # process 3
    # :
:

↓

改善後

def main():
    func1()
    func2()
    func3()

def func1():
    # process 1
    # :

def func2():
    # process 2
    # :

def func3():
    # process 3
    # :
:

例：テキスト変換スクリプト(改善後)

import sys


def main(input_file, output_file):
    input_lines = read_file(input_file)
    output_lines = convert_lines(input_lines)
    write_file(output_file, output_lines)


def read_file(input_file):
    with open(input_file, 'r') as f:
        lines = f.readlines()
    return lines


def convert_lines(input_lines):
    output_lines = [line.lower() for line in input_lines]
    return output_lines


def write_file(output_file, lines):
    with open(output_file, 'w') as f:
        f.writelines(lines)


if __name__ == '__main__':
    main(sys.argv[1], sys.argv[2])

結果

• コードが追いやすい
  • mainを見れば、①ファイルを読み込んで、②各行を変換して、③ファイルに書き込む、という全体の流れがわかる
• 適切な関数名を付けやすい
  • 各関数でやっていることが１つなので適切な名前を付けやすい
• テストがしやすい
  • 各関数でやっていることが１つなのでテストをしやすい
  • convert_linesではアウトプットの文字列リストを検証するだけでいい