質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

87.49%

Python オブジェクト指向 クラス設計について

解決済

回答 3

投稿 編集

  • 評価
  • クリップ 4
  • VIEW 4,316

score 1275

前提・実現したいこと

ここ最近オブジェクト指向関連の稚拙な質問ばかりで申し訳ないです。
クラス設計についてどうすればいいのか分からないので教えて欲しいです。

該当のソースコード

Report.py
from Dict import Dictonary

class Report:
    def __init__(self,name,comment):
        self._name = Name(name)
        self._comment = Comment(comment)

    def macthWords(self):
        dict = Dictionary()
        words = self._comment.parse()
        return [word for word in words if dict.isMatch(word)]


class Name:
    def __init__(self,name):
        self._name = name

    def isSame(self,name):
        return self._name == name

class Comment:
    def __init__(self,comment):
        self._comment = comment

    def parse(self):
        #parse処理
        return words
Dict.py
class Dictionary:
    def __init__(self):
        self._dict = self.__loadDictionary()

    def __loadDictionary(self):
        #ファイルを読み込んで辞書を作成
        return dict

    def isMatch(self,word):
        return word in self._dict

この設計の問題点

今現状のクラス図はこのような感じでしょうか
NameとCommentはラップクラスです。

           |
           |
           |
[class Report:] ------------------------[class Dictionary:]
           |
           |--------------------┐
           |                            |
[class Name:]    [class Comment:]


問題が2つあります。
①ReportクラスのmatchWordsメソッドが呼ばれるたびに辞書を作成しまう。
②単一責任の原則に反しているのではないか?(commentでparseしたものをReportクラスで扱っている為)

②に関してはラップクラスとしてのCommentの責任からは離れているので問題ないのかなとは思っているのですが分からないので教えてください。
①は何か解決策がありましたら教えてください。

これ以外にも何かおかしかったり、こうした方がいいというのがあれば教えてください。

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 3

checkベストアンサー

+1

(1) ReportクラスのmatchWordsメソッドが呼ばれるたびに辞書を作成しまう。

これはオブジェクト指向としての話題か実装テクニックの話かよくわかりませんが・・・後者だとしてコメントしてみます。

この辞書をみると「何物にも依存しない辞書」に見えるので、インスタンスのライフサイクルをどうするか考えてみてはいかがでしょうか?辞書インスタンスのライフサイクルはメソッドの呼び出し期間、インスタンスの存在期間、クラスの存在期間、アプリケーションの存在期間のいずれにするかで設計に違いがでると思います。

またあまりよいことではないかも知れませんが、Dictionaryのインスタンスをいくつ作っても辞書本体の情報はDictionaryクラスが静的に保持しているという作り方もできなくはありません。利用者は毎回インスタンスを作るのですが、実際のロードはDictionaryクラスが「必要に応じて1回だけ」にするといったトリックです。

(2) 単一責任の原則

CommentやReportが「何であって何でないか」の設計次第だと思います。例えば「コメント」というクラス名からは「単語を分解する」とか「キーワードとそれ以外を取り出す」といった機能のイメージが(自分は)わいてきません。またReportが「何をリポートする目的のものか」いいかえれば「現実世界のどういった問題をモデル化したものか」イメージがわかないので「何をどのクラスに役割分担させようとしているか」がわかりません。それがイメージしにくいと各クラスが何を意味しているかがわからないのではっきりとした意見が持てない気がします。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/06/16 19:45

    ありがとうございます。
    最後に、>pashango2さんの回答コードをみてもらいたいのですが、Commentクラスのクラス変数としてDictionaryクラスのインスタンスを保持するのはOOP的に問題ないのでしょうか?

    キャンセル

  • 2017/06/16 19:59

    pashango2さんの考え方は
    「辞書はReportに依存する情報」
    「インスタンスごとに異なるものではなく共通のもの」
    という前提での設計であり、その前提が質問者さんの意図にマッチするかどうかは別として、OOPとして自然な設計の一つだと自分は思います。

    キャンセル

  • 2017/06/16 20:00

    ありがとうございます!全て解決しました!

    キャンセル

0

最終的に以下のように使えるようになったらオブジェクト指向だと思います。

dictionary = Dictionary('file')
report = Report('name', 'comment')
words = dictionary.matchWords(report.comment.parse())

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

0

①に関して、一番簡単な方法は以下の通りです。

class Report:
    _dict = Dictionary()

    def __init__(self,name,comment):
        self._name = Name(name)
        self._comment = Comment(comment)

    def macthWords(self):
        words = self._comment.parse()
        return [word for word in words if self._dict.isMatch(word)]

コードを読む限り、実ファイルからいつも同じ辞書を作成しているようなので、クラスメンバを使います。
C++で言うところのstaticメンバですね。

②に関して言えば、どこでもいいと思います。
というのも、責任の所在を議論するにはCommentクラスの機能が少なすぎます。

その他
設計には個々の好みがあるので、確実にこうした方がいい部分だけ列挙します。

Directoryクラスは実ファイルと密結合しているし、各々のクラス同士が少し結合度が高いかなと思います。
Dependency Injectionなどで少し結合度を下げた方が良いと思います。

Pythonじゃなくて「Javaっぽいな」というのが印象です、少なくともPEP8を守った方がよいかなと思います。dictなどのビルトインクラスの名前の上書きしている部分があるので、flake8などの静的解析ツールを導入したほうがよいですよ。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2017/06/16 18:40

    なるほどです。
    すごくスッキリしていますが、これってOOP的に問題はないのですか?

    キャンセル

  • 2017/06/16 20:12 編集

    問題はありますよ、もしかしたらボロカスに言われるかもしれません。

    しかし、この方法は「これ以上ないくらいシンプル」という何にも代えがたい力があります。
    この方法で将来的に問題が出ないかもしれないのに、先回りしてシンプルさを崩す不可逆な変更をする事は愚策であると思います、不確定な未来に対しての過剰投資であり、早すぎる最適化です。
    私なら問題が発覚してから対処します、対処しやすいようにある程度の軸足を考えて組む必要がありますが・・・

    まぁ、設計は好みですので、この意見に納得できない場合はDictionaryを外部から注入してください。
    Dictionaryの寿命管理責任はReportクラスよりも上のクラスにありますので。

    キャンセル

  • 2017/06/16 20:50 編集

    上のコメントはDirectory -> Dictionaryでした、修正しました。

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 87.49%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

同じタグがついた質問を見る