共通関数化でDB取得が重複しそうになったので責務分離を見直した話
はじめに
もともとは、ある処理全体を行う func() だけを共通関数にする予定でした。
しかし実装を進める中で、func() の内部で呼んでいた sub1_func() や sub2_func() も、別の処理から単体で呼び出したいという話になりました。
そのときに気になったのが、DBに保存している固定値の取得場所です。
func() から呼ぶ場合は固定値を一度だけ取得できれば十分です。
一方で、sub1_func() や sub2_func() を単体で呼ぶ場合は、それぞれの関数でも固定値が必要になります。
今回は、共通関数化に伴ってDB取得が重複しそうになったため、固定値の取得と計算処理の責務を分けた話を整理します。
背景
今回の前提は以下です。
-
requestにはquantityが入っている -
width/heightなどの固定値はDBに保存している - 固定値は同一処理内では変わらない
-
func()は複数の処理を組み合わせて最終結果を返す -
sub1_func()/sub2_func()も単体で呼び出される可能性がある
最初は以下のような構成を想定していました。
def func(request):
fixed_value = get_fixed_value()
sub1_result = sub1_func(request, fixed_value)
sub2_result = sub2_func(request, fixed_value)
return build_result(sub1_result, sub2_result)
この時点では、固定値の取得は func() の中で一度だけ行えば問題ありませんでした。
問題になった設計
その後、sub1_func() や sub2_func() も共通関数として単体利用したい、という話になりました。
単体で呼べるようにするだけなら、各関数の中で固定値を取得する形が分かりやすいです。
def func(request):
fixed_value = get_fixed_value()
sub1_result = sub1_func(request)
sub2_result = sub2_func(request)
return build_result(sub1_result, sub2_result)
def sub1_func(request):
fixed_value = get_fixed_value()
return request.quantity * fixed_value.width
def sub2_func(request):
fixed_value = get_fixed_value()
return request.quantity * fixed_value.height
しかし、この形だと func() から呼び出したときに固定値の取得が重複します。
sub1_func() や sub2_func() を単体で呼ぶ場合には便利です。
ただ、func() のように複数の関数を組み合わせて呼ぶ場合、内部で何度も同じDB取得が発生してしまいます。
また、関数の中にDBアクセスが隠れるため、呼び出し元から処理コストが見えにくくなる点も気になりました。
改善後の設計
今回は、DB取得と計算処理を分けることにしました。
def func(request):
fixed_value = get_fixed_value()
width = calculate_width(request.quantity, fixed_value)
height = calculate_height(request.quantity, fixed_value)
return build_result(width, height)
def calculate_width(quantity, fixed_value):
return quantity * fixed_value.width
def calculate_height(quantity, fixed_value):
return quantity * fixed_value.height
func() では最初に固定値を一度だけ取得します。
その後、取得済みの fixed_value を calculate_width() や calculate_height() に渡します。
この形にすると、calculate_width() や calculate_height() はDBアクセスを行わず、受け取った値を使って計算するだけになります。
単体で呼びたい場合
calculate_width() を単体で使いたい場合は、DB取得込みのラッパー関数を別に用意します。
def calculate_width_with_db(request):
fixed_value = get_fixed_value()
return calculate_width(request.quantity, fixed_value)
def calculate_width(quantity, fixed_value):
return quantity * fixed_value.width
このように分けると、それぞれの役割が明確になります。
calculate_width:
取得済みの固定値を使って計算する
calculate_width_with_db:
DBから固定値を取得してから計算する
func:
固定値を一度だけ取得して、複数の計算処理を組み合わせる
単体利用のしやすさを残しつつ、上位関数から呼ぶ場合のDB取得重複も避けられます。
この設計で良かったこと
1. DBアクセスの場所が明確になった
修正前は、関数の中でDB取得が行われていました。
そのため、呼び出し元から見ると「この関数を呼ぶとDBにアクセスする」ということが分かりにくい状態でした。
修正後は、get_fixed_value() を呼ぶ場所が上位関数やラッパー関数に寄るため、DBアクセスのタイミングを追いやすくなりました。
2. 計算処理をテストしやすくなった
calculate_width() や calculate_height() はDBに依存しません。
そのため、テスト時には固定値を用意して渡すだけで確認できます。
class FixedValue:
def __init__(self, width, height):
self.width = width
self.height = height
def test_calculate_width():
fixed_value = FixedValue(width=10, height=20)
result = calculate_width(3, fixed_value)
assert result == 30
DBを用意しなくても計算処理を確認できるため、単体テストが書きやすくなりました。
3. 関数の責務が分かりやすくなった
修正前は、1つの関数の中で「DB取得」と「計算」の両方を行っていました。
修正後は、以下のように役割を分けています。
- DBから固定値を取得する処理
- 取得済みの固定値を使って計算する処理
- 複数の計算結果を組み合わせる処理
責務を分けることで、どの関数が何を担当しているのかを読み取りやすくなりました。
注意点
この設計では、取得済みの固定値を引数で渡す必要があります。
def calculate_width(quantity, fixed_value):
return quantity * fixed_value.width
そのため、呼び出し元は事前に fixed_value を用意しなければなりません。
単純に呼びたい場合には少し手間が増えるため、必要に応じてDB取得込みのラッパー関数を用意すると使いやすくなります。
また、渡す固定値が増えてくると、引数が多くなりすぎる可能性があります。
def calculate_price(quantity, width_master, height_master, price_master):
...
このような状態になる場合は、計算に必要な値をまとめたContextのようなオブジェクトを検討しても良さそうです。
class CalculationContext:
def __init__(self, fixed_value):
self.fixed_value = fixed_value
ただし、最初からContext化すると大げさになる場合もあります。
まずはシンプルに引数で渡し、必要になったらまとめるくらいで良いと思います。
まとめ
もともとは func() だけを共通関数にする予定でした。
しかし、内部で使っていた sub1_func() や sub2_func() も単体で呼び出す必要が出てきたことで、固定値の取得場所を見直すことになりました。
単体で呼べる便利な関数にしようとすると、関数内部でDB取得まで行いたくなります。
ただ、その設計にすると、上位関数から複数の共通関数を呼び出したときに、同じDB取得が重複する可能性があります。
今回は、DB取得込みの処理と、取得済みデータを使う計算処理を分けることで、以下の状態にしました。
- DBアクセスの場所を分かりやすくする
- 同じ固定値の取得を重複させない
- 計算処理をテストしやすくする
- 関数の責務を明確にする
共通関数を作るときは、「単体で呼べるか」だけでなく、「内部でどのような処理コストが発生するか」も意識したいと思いました。