LoginSignup
2
1

More than 1 year has passed since last update.

@h1day

「アジャイルソフトウェア開発の奥義」の19.4章をコメント通りVisitorパターンにする

Last updated at Posted at 2021-07-24

概要

ロバート・C・マーチンの「アジャイルソフトウェア開発の奥義」の19.4章。

リスト19-22のChangeUnaffiliatedtransaction.javaでは従業員が組合員かどうかで挙動を変えています。
その方法として以下の３つからダウンキャスト（ダイナミックキャスト）を選択しています。

  1. ダウンキャスト
  2. 抽象メソッド
  3. Visitorパターン

他の例も含めてC++で実装して確認してみました。

目的

typeidで分岐する処理（ダウンキャストしたり、enumを使ったりを含む）は、処理の種類が増えるたびにswitch文が各所にばら撒かれることになります。
このswitch文は「ショットガン創」とも呼ばれ実装漏れしてもコンパイルエラーにならないので、typeidを追加する変更を難しくします。

それぞれの方法がこの観点でどうなっているかを観察します。

コードの準備

今売っている第二版と違い、初版だとこの部分はC++で書かれています。「サポートページ」からダウンロードして利用します。
ただ、色々使いにくい状態だったため変更したレポジトリを作りました。

注目しているそれぞれの実装にタグ付けしています。

  • 19.4Downcast(ダウンキャスト、ダイナミックキャスト)

  • 19.4VirtualFunctions（仮想関数、抽象メソッド）

  • 19.4VisitorPattern（Visitorパターン）

該当部分の確認(ダウンキャスト、ダイナミックキャスト)

問題の個所は、従業員の所属を変更する処理から呼び出されているRecordMembershipです。

ChangeAffiliationTransaction.cpp
void ChangeAffiliationTransaction::Change(Employee& e)
{
    RecordMembership(&e);
    e.SetAffiliation(GetAffiliation());
}

ChangeUnaffiliatedTransaction（所属の削除処理）のRecordMembershipの中では、e->GetAffiliation()で従業員の現在の所属を入手して、そのクラスが組合(UnionAffiliation)であるなら削除処理をします。そうでない(NoAffiliation)なら何もしません。

ChangeUnaffiliatedTransaction.cpp
#include "ChangeUnaffiliatedTransaction.h"
#include "NoAffiliation.h"
#include "UnionAffiliation.h"
#include "PayrollDatabase.h"

extern PayrollDatabase GpayrollDatabase;

// 省略
void ChangeUnaffiliatedTransaction::RecordMembership(Employee* e)
{
  Affiliation* af = e->GetAffiliation();
  if (UnionAffiliation* uf = dynamic_cast<UnionAffiliation*>(af)) {
    int memberId = uf->GetMemberId();
    GpayrollDatabase.RemoveUnionMember(memberId);
  }
}

確かにdynamic_castを使っています。これで切り替える対象のクラス(typeid)が増えるとswitch文になります。
このような実装の個所が増えると実装漏れが発生し対処できなくなってきます。

本文中では以下のように述べています。

この設計に十分満足しているとは言えない。ChangeUnaffiliatedTransactionがUnionAffiliationについて知らなければならないということが気になるからだ。
これを解決するひとつの方法は、抽象メソッドRecordMembershipとEraseMembershipをAffiliationクラスの中に置くやり方だ。
しかしながらそうしてしまうと、UnionAffiliationとNoAffiliationがPayrollDatabaseのことを知らなければならなくなってしまうだろう。
私としては、そうなってしまうのも気に入らない。

さらに脚注でこのように言っています。

第28章のVisitorパターンを使ってこの問題を解消することもできる。しかし、それはちょっとやりすぎだろう。

仮想関数、抽象メソッド

上で著者が挙げている抽象メソッド（仮想関数）RecordMembershipとEraseMembershipをAffiliationクラスの中に置くやり方を想像して書きました。
現状の所属（afBefore ）に応じて一旦削除処理をして、次の所属（afAfter ）に応じて必要ならデータベースに登録します。

ChangeAffiliationTransaction.cpp

// 省略

void ChangeAffiliationTransaction::Change(Employee& e)
{
    auto* afBefore = e.GetAffiliation();
    afBefore->EraseMembership();

    auto* afAfter = GetAffiliation();
    afAfter->RecordMembership(e);
    e.SetAffiliation(afAfter);
}

クラス(typeid)が増えても純粋仮想関数のオーバーライドを強制できるので、コンパイル時に実装漏れに気づけます。
しかし、組合への所属を表すUnionAffiliationは、データベースへの登録処理を担うことになったので、g_payrollDatabaseを知らなくてはならなくなりました。別のデータベースやメールを送る処理など、仕様が増えるたびに依存先が増えていきます。確かにイマイチですね。

UnionAffiliation.cpp
#include "UnionAffiliation.h"
// 省略
#include "PayrollDatabase.h" //todo: bad dependency

extern PayrollDatabase g_payrollDatabase; //todo: bad dependency

// 省略

void UnionAffiliation::EraseMembership()
{
    const int memberId = this->GetMemberId();
    g_payrollDatabase.RemoveUnionMember(memberId);
}

void UnionAffiliation::RecordMembership(Employee& e)
{
    g_payrollDatabase.AddUnionMember(itsMemberId, &e);
}

Visitorパターン

Affiliationクラスの中に用途毎の仮想関数を置く代わりにVisitorパターンのAcceptを置きます。これも純粋仮想関数です。
ここに実行したい処理に応じてAffiliationVisitorを継承したAffiliationEraserとAffiliationRecorderを注入します。

ChangeAffiliationTransaction.cpp

// 省略

void ChangeAffiliationTransaction::Change(Employee& e)
{
    auto ve = AffiliationEraser();
    auto* afBefore = e.GetAffiliation();
    afBefore->Accept(ve);

    auto vr = AffiliationRecorder(e);
    auto* afAfter = GetAffiliation();
    afAfter->Accept(vr);
    e.SetAffiliation(afAfter);
}

UnionAffiliationはデータベースg_payrollDatabaseではなく、処理のインターフェースであるAffiliationVisitorに依存するようになりました。

UnionAffiliation.cpp
#include "UnionAffiliation.h"
// 省略
#include "AffiliationVisitor.h"

// 省略

void UnionAffiliation::Accept(AffiliationVisitor& v)
{
    v.visit(*this);
}

AffiliationVisitorを継承した処理を表すクラスには、各typeid向けの処理を実装していくことになります。

AffiliationRecorder.cpp
#include "AffiliationRecorder.h"
#include "UnionAffiliation.h"
#include "PayrollDatabase.h"

extern PayrollDatabase g_payrollDatabase;

AffiliationRecorder::AffiliationRecorder(Employee& e)
    : AffiliationVisitor(),
      itsEmployee(&e)
{
}

void AffiliationRecorder::visit(NoAffiliation& a)
{
    //do nothing
}

void AffiliationRecorder::visit(UnionAffiliation& a)
{
    const int memberId = a.GetMemberId();
    g_payrollDatabase.RemoveUnionMember(memberId);
    g_payrollDatabase.AddUnionMember(memberId, itsEmployee);
}

実装するファイルが処理ごとに別々でいかにも実装漏れが起きそうですが、大丈夫です。
新しいtypeidを追加するときには、継承しているAffiliationによってAcceptの実装が強制されます。
デザインパターンに従って、処理内容にv.visit(*this)を実装すると新たな引数のオーバーロードで、新しい純粋仮想関数が作られます。AffiliationVisitorを継承した各処理はすべての純粋仮想関数をオーバーロードしなくてはいけないので、実装を忘れるとコンパイルエラーになります。
これにより、実装漏れを防げます。

まとめ

switch文がばら撒かれる問題に対して、

  • 本当に修正箇所が1か所しかないならダウンキャスト（ダイナミックキャスト）がシンプル。ただし、実行するまで実装漏れに気付けないので、場所が増えたら辛い。
  • 仮想関数を使うと実装を強制できる。処理の種類が増える毎に各クラスが依存する対象が増えていく。
  • Visitorパターンは、実装漏れを防ぎながら依存関係は各ビジターに分離できる。処理を追加してもacceptを持つクラスを変更しないので、オープン・クローズドの原則にも適合する。しかし、「１＋処理の種類」だけクラスが増えていき大げさ。そもそもこのデザインパターンが分かりにくい。

おまけ：Visitorパターンと処理の引数

参考にしている書籍では取り上げられてなくて混乱したのでメモします。
抽象メソッドを配置したとき、引数eが「ない/ある」の2種類の処理がありました。

    af->EraseMembership();
    af->RecordMembership(e);

Visitorパターンでは処理の引数eはVisitorのコンストラクタに入力しています。
これにより様々な引数の処理を扱えます。

    auto ve = AffiliationEraser();
    afBefore->Accept(ve);

    auto vr = AffiliationRecorder(e);
    afAfter->Accept(vr);

参考

2
1
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
2
1