192
100

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 5 years have passed since last update.

デメテルの法則を厳密に守るにはどうすればいいの?

Posted at

概要

デメテルの法則(最小知識の原則)を厳密に守ろうとするのってすごく難しいよねってお話です。
色々書いて「この方がいいのでは」というのを最後のほうにも書きましたが、そのやり方が正しいのかわからない・・・誰か教えて(´・ω・`)

シミュレーション

この投稿では以下の実装をしようとしているという前提で話を進めていきます。

関わっているクラス

  • User :あるアプリケーションのユーザーを表現しているクラス
  • BasicInfo :ユーザーの基本情報(アドレスとか名前とかがあるイメージ)
  • Configuration :ユーザーが設定した情報
  • Addon :アドオンを有効にしてるかどうか、みたいな感じ
  • UserService : Userに関してのビジネスロジックを扱うクラス

クラスの関係

image.png

やろうとしていること

UserService内でユーザー設定の「Addon」が有効かどうかを確認し、有効ならAddonの一覧を取得するGet通信を行う

最初考えたコード

if (User.BasicInfo.Configuration.Addon.IsEnabled) {
    // Get通信処理
}

何が嫌なのか

デメテルの法則に反していて、UserServiceが色々と知りすぎているというのがつらい(´・ω・`)

デメテルの法則って?

WikiPediaより

デメテルの法則 (Law of Demeter, LoD) または最小知識の原則 (Principle of Least Knowledge) とは、ソフトウェアの設計、特にオブジェクト指向プログラムの設計におけるガイドラインである。 このガイドラインは1987年の末にかけてノースイースタン大学で作成された。簡潔に言うと「直接の友達とだけ話すこと」と要約できる。基本的な考え方は、任意のオブジェクトが自分以外(サブコンポーネント含む)の構造やプロパティに対して持っている仮定を最小限にすべきであるという点にある。

もっと簡単にいうと「😎おっとお前さん内部の事情を知りすぎてるぜ」みたいなことですね。ギャングの世界であれプログラミングの世界であれ、あまり知らない方が幸せに生きていけます。

具体的に何がよくないのか

最初のコードも、「動けばよかろうなのだ」でいうと何も悪くないです。ただ結合度が高くなるため、テストを書いたり改修したりする際に不都合が生じてきます。

結合度とは、簡単に言えば「そのクラスが何を知り、何を利用しているか」の指針です。

上のコードは、UserServiceが

  1. UserがBasicInfoというプロパティを持ってることを知っており、呼び出してます
  2. BasicInfoがConfigurationというプロパティを持ってることを知っており、呼び出してます
  3. ConfigurationがAddonというプロパティを持ってることを知っており、呼び出してます
  4. AddonがIsEnabledというプロパティを持ってることを知っており、呼び出してます

呼び出せるということは 知っている ということです。
わずか1行の処理で、UserServiceが思った以上に色々な事情を知ってることが露呈しています。
こんなクラス、ギャングの世界ならいつ生き埋めにされても文句言えませんね? 😎

お前は色々と知りすぎた

ギャングオブフォー曰く、プログラミングの世界もあまり知りすぎない方がいいとされてます(関心の分離
なぜ知りすぎるとよくないのかというと、以下の弊害があるからです。

弊害1. ユニットテストが大変

ユニットテストを行う際、一般的には自分とは関係のないものはモックにして常に同じ値を返すことで仕様の担保を行います。その際に関係しているクラスが多い(結合度が高い)と、1回のテストでもたくさんのモックを作る必要がでてきます。

めんどいよね?

弊害2. 改修が大変

結合度が高いということは、知っている(依存している)クラスの仕様が変わるともろに影響を受けることを意味しています。

たとえば、BasicInfoからConfigurationを無くそうと思ったら、UserService も修正する必要がでてきます。あるいは別のものを返すようになった場合にも、やっぱり影響を受けます。
これは、依存しているクラスの仕様を変えようとするたびに、UserSerivceに影響がないかも調べる必要がでてくるということです。

めんどいよね?

しかも、依存してるクラスを変更したら、UserServiceのユニットテストも修正をしないといけません。

そんなの忘れるよね?

じゃあどうするのか

ここからが本題です。上のは嫌だから変えたいのですが、具体的にどのように変更することが正解なのでしょう。
とりあえず、思いつくままに変えてみます。

1. とりあえずドットをなくす

var basicInfo = User.BasicInfo;
var configuration = basicInfo.Configuration;
var addon = configuration.Addon;
if (addon.IsEnabled) {
    // API送信処理
}

本質は何も変わってませんね。ありがとうございました。

2. UserクラスにAddonが有効かどうか調べるメソッドを作る

// それぞれにプロパティを作る
class Configuration {
   public bool IsAddonEnabled {
       get {
           return Addon.IsEnabled;
       }
   } 
}

class BasicInfo {
   public bool IsAddonEnabled {
       get {
           return Configuration.IsAddonEnabled;
       }
   } 
}

class User {
   public bool IsAddonEnabled {
       get {
           return BasicInfo.IsAddonEnabled;
       }
   }
}

// 使う側
if (User.IsAddonEnabled) {
    // API送信処理
}

これで、デメテルの法則の反さないことはできますね。
でも、同じようにまたAddonとかから別のプロパティを見ないといけない時には同じことをしないといけないのでしょうか。

ちょっと「生きてて楽しいの?」って気分になってきますね。

3. 設計を考え直す

そもそも、今の作りだと BasicInfo(基本情報) の中にConfigurationというのが入ってますが、これははたしてあるべき形なのでしょうか。

BasicInfo自体をなくすというのもアリかもしれませんし、 基本情報 を表しているのだとしたら基本的にそんなに変更はしないはずの領域で、少なくとも設定のようにころころ変更されるものを持たせるのは適さないかもしれません。

たとえば、以下のように持たせ方を変更します。

image.png

すると、以下のようにちょっと減らせますね。

if (User.Configuration.Addon.IsEnabled) {
    // API送信処理
}

でも依然として依存は高い状態です。

もうちょい進めてみる

そもそもAddonもUserに直接持たせるというのはどうでしょうか。いっそのことやめてみます。

image.png

if (User.Addon.IsEnabled) {
    // API送信処理
}

まだ知りすぎとるのう。。。

そうじゃ、もういっそのことUserServiceにAddonを持たせるじゃろ?
すると、こうできるじゃろ?

if (Addon.IsEnabled) {
    // API送信処理
}

完璧じゃ!!

image.png

・・・おや、また UserServiceは4クラスに依存する 形に戻りましたね?

違う、そうじゃない

この敗因はどこにあるのでしょう。
そしてぼくは一体どこへ向かおうとしているのでしょう。

実のところ、正解に近いところまで行っていたのではないかという気もしています。
設計を見直す というのは間違えていないように思えます。ただぼくは デメテルの怒りを鎮めたい という単純な理由で持ち方を変えようとした結果、設計の目的を見失ってしまったのかもしれません。

それに、どうもぼくはまだ 手続き型 のレビューをしているところにも敗因があるような気がしています。
つまりは

if (User.BasicInfo.Configuration.Addon.IsEnabled) {
    // API送信処理
}

そもそもこの実装自体を疑うべきなのかもしれません。
でも、どのようにしたらいいのか。

そもそもUser.BasicInfoにConfigurationを持たせてる理由は?

もう一度設計について考えてみます。
(ちなみに今更ですが、この投稿でいう設計とは主にクラス設計を指してます)

クラスは、一般的にはメンバ変数とメソッドを持ちます。ではメンバ変数って何をしているかというと、 そのオブジェクトの属性を表している という一言に尽きます。また、 デメテルの法則的表現 をすると、属性は以下であるとも言えそうです。

「オブジェクトは、自分の属性のことは知っている」

たとえば、 User というクラスには、ユーザーが知っているべき属性を入れるのが良いような気がします。

  • 自分の名前
  • メールアドレスとかパスワードとか
  • 最後にいつログインしたか

image.png

・・・あれ、 BasicInfoってなんだっけ?

もう一度設計を見直す

ユーザーの 属性 という観点で見たとき、BasicInfoってよくわからない子ですね。それによくよく考えてみると、何を表したいものなのかもよくわからない。なぜユーザーに直にデータをもたせるのではなく、わざわざ BasicInfo を噛ませているのか。

もちろんのあたりはシステムの仕様にも依存するので一概に「間違えてる」とも言えないでしょう。ただ再度 デメテル的表現 をすると、今の持ち方は以下を表現していると言い換えることができます。

  • Userは、BasicInfoのことは知っている
  • Userは、自分の名前やEmailを知らない(BasicInfoが知ってることは知ってる)
  • Userは、自分の設定(Configuration)を知らない(BasicInfoが知ってることは知ってる)

なんか、やっぱりおかしい気がしてきました。
知ってる という観点だと、以下の方が正しいように思えます。

image.png

ちょっとまってほしい、属性的に合ってるのか

ここで、もう一度「メンバ変数=属性」という点で考えてみます。
属性とは その事物が持っている性質 を現しています。そういう点で、 UserがConfigurationを持つというのは正しいでしょうか。

その設定は「ユーザーの設定」ではなく、正しくは 「ユーザーが調節したユーザー用のシステム設定」 です。たとえば背景色を変更したからといって、ぼく自身が緑色になったり虹色になったりするのではなく、システムの見栄えが変わります。
その観点でいうと、「User」と「Configuration」は分離して、必要な時だけ用意するという形でも良いのではないでしょうか。

image.png

使う時にはこんな感じになります。だいぶデメテル的にいい感じになってきました。

if (Configuration.Addon.IsEnabled) {
    // API送信処理
}

尋ねるな、命じろ

ただ、まって下さい。だいぶデメテル的満足に近づいてはいますが、まだ違反してます。これにはデメテルもぷんぷんです。
そもそも、この処理は 手続き型 な感じです。状態を尋ねてその結果で処理する というより、オブジェクトそのものに処理させる 方が良いかもしれません。

// 使う側 (API接続用のを渡してあげるイメージ)
Configuration.receiveAddonList(apiClient);

// Configuration側の定義
public AddonList receiveAddonList(ApiClient client) {
    return Addon.receiveList(client);
}

// Addon側の定義
public AddonList receiveList(ApiClient client) {
    if (!IsEnabled) {
        // 有効でなければ空を返すとか
        return new AddonList();
    }
    // API実行処理
}

これでようやくデメテルとの約束を果たすことができました。デメテルもにっこりでしょう。
(Addonの一覧を受け取るためのサービスクラスを新たに作るなどするほうが良いかもですが、方針としてはこんな感じ)

まとめ

くどくど書いてきましたが、つまりは デメテルの法則 を厳密に守ろうと思うなら、付け焼き刃的なやり方は無理で、設計から考え直さないと無理なんじゃないかなって思ってます。

そして設計の際には以下の2つの観点を入れました。

  • オブジェクトは、自分に定義されている属性のことは知っており、定義されていないことは知らない
  • 尋ねるな、命じろ

とはいえ、結構これを貫くのは大変なんじゃないかと思ってて、他にコツとかないのかって気になってます(´・ω・`)

設計からやりなおすって、ある程度開発が進んでたら厳しいよねぇ。。。
しかも実際のシステム開発の現場では

if (User.BasicInfo.Configuration.Addon.IsEnabled) {
    // 何かの処理
}

こういうデメテルぷんぷん事案を結構よく見かけますもん・・・。

これを見かけた時点でコードの熱エントロピーが増大しているという警告で、リファクタリングしろってサインなのかもしれませんが・・・そう思っといた方がいいのかな・・・?

192
100
1

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
192
100

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?