💭

関数っていつどう分けたらいい?最近ぼくが意識していること

に公開
1

はじめに

「関数をどのタイミングで分けるべきか?」という問いは、初歩的なテーマに思えるかもしれません。しかし実際には、コードの読みやすさや保守性、さらにはチーム全体の生産性にも大きく関わる、非常に奥の深いトピックだと思います。

ここ半年ほど、社内のアーキテクトに相談しながら、自分なりに「関数を分ける判断」について意識的に考えるようになりました。その中で得た気づきや学びを整理し、備忘も兼ねてこの記事にまとめてみたいと思います。

サンプルコードはPythonで記述しています。
(型ヒントや構文がシンプルため)

これまでの考え方

私自身、以前は主に以下の基準で関数を分けていました:

  • コードが長くなってきたとき
  • 責務が複数あるように見えたとき

一見もっともらしいこれらの基準にも、実際にプロダクト開発を進めていく中でいくつかの“落とし穴”があると感じるようになりました。

「コードが長いから関数に分ける」

コードが長いという理由だけで安易に関数を分割すると、1つの処理の流れを追うために何度も private 関数へジャンプしなければならず、かえって読みにくくなることがあります。個人的には、同じファイル内でジャンプを繰り返すコードは、処理の全体像がつかみにくくなるためあまり好みではありません。

※このトピックについて深く考え始めたきっかけも、あるプログラムを読んでいるときに「1つの処理を読むためだけになぜこんなにメソッドジャンプが必要なのか」と疑問を抱いたことでした。

以下のコードは、1つのビジネス処理(注文処理)を読むためにメソッドジャンプを何度も行わなければならない構造です。(極端な例ですが...)

class OrderProcessor:

    def handle(self, order: Order) -> None:
        self._initialize()
        self._validate_order(order)
        self._apply_discount(order)
        self._calculate_shipping(order)
        self._persist_order(order)
        self._notify_customer(order)
        self._log_completion()

    def _initialize(self) -> None:
        print("Initializing...")

    def _validate_order(self, order: Order) -> None:
        self._check_null(order)
        self._check_items(order)
        self._check_customer(order)

    def _check_null(self, order: Order) -> None:
        if order is None:
            raise ValueError("Order must not be null")

    def _check_items(self, order: Order) -> None:
        if not order.items:
            raise ValueError("Order must have at least one item")

    def _check_customer(self, order: Order) -> None:
        if order.customer is None:
            raise ValueError("Customer must be specified")

    def _apply_discount(self, order: Order) -> None:
        if order.total > 10000:
            order.discount = 1000
        else:
            order.discount = 0

    def _calculate_shipping(self, order: Order) -> None:
        if order.is_express:
            order.shipping_fee = 1500
        else:
            order.shipping_fee = 500

    def _persist_order(self, order: Order) -> None:
        self._open_connection()
        self._save_to_database(order)
        self._close_connection()

    def _open_connection(self) -> None:
        print("DB connection opened")

    def _save_to_database(self, order: Order) -> None:
        print(f"Saving order for customer: {order.customer.name}")

    def _close_connection(self) -> None:
        print("DB connection closed")

    def _notify_customer(self, order: Order) -> None:
        print(f"Sending confirmation email to: {order.customer.email}")

    def _log_completion(self) -> None:
        print("Order processing complete")

「責務が複数に見えるから関数に分ける」

表面的には関数単位で責務を分けたつもりでも、システム全体を俯瞰して見ると、実は単一の処理をステップごとに分割しただけだったというケースもあります。

たとえば、ある Usecase 内で 3 つの処理が呼び出されていたとしても、それぞれが連続する 1 つの業務処理(例:ユーザー登録)を構成しているだけであれば、無理に分けないほうが理解しやすいこともあります。

class Usecase:

    def handle(self, request: UserRequest) -> None:
        self._validate(request)
        user = self._convert(request)
        self._save(user)

    def _validate(self, request: UserRequest) -> None:
        if not request.email:
            raise ValueError("Email is required")
        if "@" not in request.email:
            raise ValueError("Invalid email format")

    def _convert(self, request: UserRequest) -> User:
        user = User()
        user.email = request.email
        user.name = request.name
        return user

    def _save(self, user: User) -> None:
        print(f"Saving user: {user.email}")

ここからは、私が関数を分けるときに今意識していることを言語化してみたいと思います。

関数を分けてみているタイミング

1. ビジネス上、後から修正を行う可能性がある処理のとき

たとえば、商品の合計金額から送料を算出するロジックがあるとします。

以下のように handle メソッドにすべての処理を詰め込むと、一見綺麗にまとまっているように見えますが、将来的に変更が入ると影響範囲が広がってしまいます。

class ShippingFeeUsecase:

    def handle(self, order: Order) -> None:
        item_total = sum(item.price * item.quantity for item in order.items)

        if order.is_express:
            shipping_fee = 1200
        elif item_total > 10000:
            shipping_fee = 0
        else:
            shipping_fee = 500

        total = item_total + shipping_fee
        print(f"合計金額: {total}円(送料: {shipping_fee}円)")

ただビジネス要件が変わる可能性は大いにありえます。
たとえば、昨今のエネルギー価格や物流費の高騰などにより、送料の計算はビジネス的に頻繁に変更される可能性があります
そのため、以下のように送料の計算を関数に切り出すことで、保守性と意図の明確さが大きく向上します。

また「ビジネス上、変更可能性があるよ」ということを他の実装者にコードベースで伝えることができます。

class ShippingFeeUseCase:

    def handle(self, order: Order) -> None:
        # 商品合計を計算
        item_total = 0
        for item in order.items:
            item_total += item.price * item.quantity

        # 送料計算をメソッドに分離
        shipping_fee = self._calculate_shipping_fee(order.is_express, item_total)

        total = item_total + shipping_fee

        print(f"合計金額: {total}円(送料: {shipping_fee}円)")

    # 送料計算の処理
    def _calculate_shipping_fee(self, is_express: bool, item_total: int) -> int:
        if is_express:
            return 1200
        elif item_total > 10000:
            return 0
        else:
            return 500

2. 何度も繰り返していて、関数全体に関わる処理のとき

何度も再利用されるような処理でかつ、他の機能やAPIにも使えそうな処理は関数として分離しておくことで再利用性が高まります。

例えば以下の注文処理のバリデーション処理があったとします。
このバリデーション処理は、同じAPIの別の注文機能だけでなく、他のAPIでも再利用される可能性があります。そのため、共通の関数として切り出しておくのが良いかと思われます。

def handle(order: Order):
    if order is None:
        raise ValueError("Order is null")
    if not order.items:
        raise ValueError("No items in order")
    if order.customer is None:
        raise ValueError("Customer is null")

    # 本処理
    print("処理を実行します...")

そのときに以下のようにvalidateOrderを分けることで例えば新しいAPIを作るときにそのまま処理を移植できたりするかもしれません。

class OrderUseCase:

    def handle(self, order: Order) -> None:
        self._validate_order(order)
        
        # 本処理
        print("処理を実行します...")

    def _validate_order(self, order: Order) -> None:
        if order is None:
            raise ValueError("Order is null")
        if not order.items:
            raise ValueError("No items in order")
        if order.customer is None:
            raise ValueError("Customer is null")

将来的に注文APIが増えた場合でも、この _validate_order をそのまま使い回すことができます。

3. ロジックの単体テストを書きたいとき

ロジックの単体テストを書きたいときも、関数に分けるタイミングだと考えています。

ただし、private関数に分けてもテストはできないため、私は別のクラスを作ってロジックを記述しています。
たとえば、先ほどの商品合計の計算に加えて、消費税の計算も行うとします。

消費税の計算を間違えると、ビジネスに大きな影響を与えかねません。
このような場合、私はロジックを別クラスに切り出して独立させ、その上で単体テストを書き、ロジックが正常に動作することを担保するようにしています。

import math

class ShippingFeeUseCase:

    def handle(self, order: Order) -> None:
        # 商品合計の計算
        item_total = 0
        for item in order.items:
            item_total += item.price * item.quantity

        # 送料計算
        shipping_fee = self._calculate_shipping_fee(order.is_express, item_total)

        # 消費税計算(外部のHelper関数)
        tax = TaxHelper.calculate_tax(item_total + shipping_fee)

        total = item_total + shipping_fee + tax

        print(f"小計: {item_total}円")
        print(f"送料: {shipping_fee}円")
        print(f"消費税: {tax}円")
        print(f"合計金額: {total}円")

    def _calculate_shipping_fee(self, is_express: bool, item_total: int) -> int:
        if is_express:
            return 1200
        elif item_total > 10000:
            return 0
        else:
            return 500

class TaxHelper:
    TAX_RATE = 0.1  # 10%

    // 消費税の計算(これでロジック単体のテストが書ける)
    @staticmethod
    def calculate_tax(amount: int) -> int:
        return math.floor(amount * TaxHelper.TAX_RATE)

このように関数を外部に切り出すことで、ロジック単体に対して単体テストを書くことができるようになります

関数に分けないほうがいいかもしれないとき

冒頭にコードを載せましたが、以下の状態になっている場合には、関数の分離を再考した方がよいかもしれません。

  • 「処理が長いから」という理由だけで関数を分離する場合
  • 意味のない関数化が多く、メソッド間のジャンプが頻発してコードの読み手に負荷をかける場合

いま実践してみている関数の分け方

私自身が今、関数を分ける際に最も意識していることは、参照透過性を持たせることです。
(これは社内のアーキテクトから教わった考え方でもあります)

参照透過性とは?

同じ入力を与えれば、必ず同じ出力が返ってくる関数

※ 日本語の情報もありますが、referential transparencyで英語検索したほうがより詳しくてわかりやすい記事に出会える印象です。

❌ 参照透過でない例:

def generate_order_id() -> str:
    return str(uuid.uuid4())

呼び出すたびに結果が変わるため、参照透過ではありません。

✅ 参照透過な関数:

def calculate_tax(amount: int) -> int:
    return int(amount * 0.1)

この関数は、同じ amount を渡せば常に同じ税額が返ってきます。

アーキテクトからは「main関数にペタッと貼ってすぐ試せるコードが良い関数」とも教わりました。

参照透過性を持たせることで以下のメリットがあります:

  • ✅ ユニットテストが容易(入力→出力が明確)
  • ✅ 再利用しやすい
  • ✅ 副作用がなく安全
  • ✅ 他の処理への移植もしやすい

リファクタリング例:非参照透過な関数からの改善

以下のように、order オブジェクトに依存する送料計算関数は、参照透過ではありません。

class ShippingFeeUseCase:

    def __init__(self):
        self._shipping_fee_log = []

    def handle(self, order: Order) -> None:
        item_total = sum(item.price * item.quantity for item in order.items)
        shipping_fee = self._calculate_shipping_fee(order)
        total = item_total + shipping_fee
        print(f"合計金額: {total}円(送料: {shipping_fee}円)")

    # この関数はorderに依存している
    def _calculate_shipping_fee(self, order: Order) -> int:
        item_total = sum(item.price * item.quantity for item in order.items)
        if order.is_express:
            return 1200
        elif item_total > 10000:
            return 0
        else:
            return 500

このような書き方をすると、関数の使いまわしが難しくなり、order の中身が変わると関数自体も作り直さなければならなくなります。

改善後の、参照透過な関数の形がこちらです。

class ShippingFeeUseCase:

    def handle(self, order: Order) -> None:
        item_total = sum(item.price * item.quantity for item in order.items)
        shipping_fee = self._calculate_shipping_fee(order.is_express, item_total)
        total = item_total + shipping_fee
        print(f"合計金額: {total}円(送料: {shipping_fee}円)")

    # 送料計算の処理
    # 引数はbooleanとintを受け取る
    def _calculate_shipping_fee(self, is_express: bool, item_total: int) -> int:
        if is_express:
            return 1200
        elif item_total > 10000:
            return 0
        else:
            return 500

こうすることで、同じis_expressフラグとitem_totalを渡せば、常に同じ送料が返ってくるようになります。つまり同じ入力を与えれば、必ず同じ出力が返ってくることを担保できています。

このように、関数の出力が入力のみに依存する構造にすることで、保守性・可読性・拡張性が大きく向上します。

まとめ

今回は、コードを書くときに関数をどう設計するかについて、普段意識していることを言語化してみました。

  • 関数を分けてみているタイミング
  • 分けないほうがよいかもしれない場合
  • 分け方の基準として参照透過性を意識すること

ただ、一番大事なのは「コードの1行1行を常に考えながら書き続け、自分の考えをアップデートし続けること」なのではないかと、最近強く感じています。
AIによって、とりあえず動くコードなら誰でも書ける時代だからこそ、運用しやすいコードを考え続けることが、開発者の仕事になるのかもしれません。

常に考え続けて異なる考え方に変わったら、また記事を書こうと思います。

これはあくまで私の一意見です。皆さんのご意見もぜひ聞かせてください!!

Discussion

ねづねづ

https://zenn.dev/plan_b/articles/47ec06ed30aff0 によせて

興味深く読ませていただきました。
ただ一点、参照透過性を意識したリファクタリングとして挙げられている_calculate_shipping_feeの例に引っかかりを覚えました。

ポイントは大別して二つあると考えているのですが、

1. _calculate_shipping_feeは元から参照透過であるはず

Orderクラスを受け取る設計であっても同じ値――例えばOrder(is_express=True, item_total=10_000)――を与える限り、このメソッドは常に同じ値を返しますよね。

2. 参照透過性とは別に、_calculate_shipping_feeにはOrder自体を引数として渡す方が好ましいカプセル化ではないか

これは記事中の下記に対する反論ともいえるのですが、

order の中身が変わると関数自体も作り直さなければならなくなります。

むしろ、Orderクラスの実装が変わっても _calculate_shipping_feeさえ作り直せばよいのです。
例えば、配送方法が 通常/特急 の2種類から エコノミー/通常/特急 の3種類に増え、Orderクラスを下記のように変えたとします。

class DeliveryOption(Enum):
    NORMAL = auto()
    EXPRESS = auto()
    ECONOMY = auto()


@dataclass(frozen=True)
class Order:
    delivery_option: DeliveryOption
    items: Tuple[Item]

    @property
    def item_total(self) -> int:
        return sum(item.price * item.quantity for item in self.items)

_calculate_shipping_feeOrderクラスのインスタンスを引数に取る場合、このメソッドの中身だけ変えれば呼び出し側は変えずに済みます。

    def _calculate_shipping_fee(self, order: Order) -> int:
        free_shipping = order.item_total > 10000
        match(order.delivery_option):
            case DeliveryOption.EXPRESS:
                return 1200
            case DeliveryOption.NORMAL:
                return 0 if free_shipping else 500
            case DeliveryOption.ECONOMY:
                return 0 if free_shipping else 200

一方、_calculate_shipping_feeis_expressitem_totalとを受け取る設計だったなら、_calculate_shipping_feeだけでなく、handleや他の使用箇所すべてを修正する必要があります。

    def handle(self, order: Order) -> None:
        item_total = sum(item.price * item.quantity for item in order.items)
        shipping_fee = self._calculate_shipping_fee(
            order.delivery_option,  # <- ここも変更が必要
            item_total,
        )
        total = item_total + shipping_fee
        print(f"合計金額: {total}円(送料: {shipping_fee}円)")


    def _calculate_shipping_fee(
        self, delivery_option: DeliveryOption, item_total: int
    ) -> int:
        free_shipping = item_total > 10000
        match delivery_option:
            case DeliveryOption.EXPRESS:
                return 1200
            case DeliveryOption.NORMAL:
                return 0 if free_shipping else 500
            case DeliveryOption.ECONOMY:
                return 0 if free_shipping else 200

ではOrder自体ではなくそのメンバーを直接渡す方が良いケースはないかというと、この例ならば商品合計額の算出を関数に切り出す場合などでしょうかね。

@dataclass(frozen=True)
class Order:
    delivery_option: DeliveryOption
    items: Tuple[Item]

    @property
    def item_total(self) -> int:
        return calc_total_price(self.items)  # これは単なるシンタックスシュガー

@dataclass
class Cart:
    items: List[Item]
    ...

def calc_total_price(items: Iterable[Item]) -> int:
    return sum(item.price * item.quantity for item in items)

order: Order
cart: Cart

calc_total_price(order.items)
calc_total_price(cart.items)    # 使い回せる

あくまで私見ですが、こういう見解もあるということで…お目汚し失礼しました。