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

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

ただいまの
回答率

88.23%

リファクタリングでクラス分割

解決済

回答 2

投稿

  • 評価
  • クリップ 1
  • VIEW 2,103

cancat

score 249

こんにちは。 
Windows10でC#のアプリケーションを開発しています。 
Visual Studio 2017 Communityを使っています。 

前提・実現したいこと

サブのクラスをわけて3つにしようと考えています。
ひとつはメインA。ふたつは関連し合うサブのクラスB,Cです。
分けるのはBをBとCとにします。
このとき、AからBとCを使うときで悩んでいます。
A-B-Cと使うのがよいのか、
A-B
-C
と使うのがよいのか、です。

具体的には、
A.Form(UI)
B.メール送信(SendMail)
C.メールアドレス管理(MailAddress)
のクラスです。
従来、UIからメール送信のみを行っていました。

ふと、メールアドレスの管理のみを別立てのクラスに分離にすれば、UIでメールアドレス管理しやすいかもと考えたのです。
とすると、
A-B
-C
と別立てにするほうがよい気がしたのです。

ただしこうすると、従来は、UIからは、

該当のソースコード

SendMail sendmail = new SendMail();
sendmail.Send("宛先名", "メール本文");

でよかったのが、

MailAddress address = new MailAddress();
address.Name = address.Search("宛先名");

SendMail sendmail = new SendMail();
sendmail.Send(address, "メール本文");

のように、sendmailの引数に、新設したMailAddressクラスを渡す必要が出てきます。
こうすると、B(SendMail)からC(MailAddress)を分離するだけでなく、UIからSendMailを呼び出している部分すべてを改変する必要が出てきます。それはちょっと手間だなぁと考えています。

こういうリファクタリングって、どのようにするものでしょうか?
クラスを分離するのだから、UIから呼び出している部分もすべて改良すべきでしょうか?
ご意見賜りたく。

補足情報(言語/FW/ツール等のバージョンなど)

Microsoft Visual Studio Community 2017
Version 15.0.26228.9 D15RTWSVC
Microsoft .NET Framework
Version 4.6.01586

です。 
よろしくお願いします。

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

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

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

    クリップを取り消します

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

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

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

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

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

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

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

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

    質問の評価を下げる

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

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

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

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

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

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

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

    詳細な説明はこちら

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

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

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

回答 2

checkベストアンサー

+2

失礼します。今登録したばかりなので、
teratailの文化に馴染めていなかったらごめんなさい。

A,B,Cは独立した機能ですので、それぞれに継承関係は無いほうがいいです。
そして、恐らく質問はクラスの継承をどのようにしようかと悩んでいるのではなく、
どのクラスがどのクラスを使ったらよいかで悩んでいるのだと思います。

結論から言うと最後の

A-B
 -C


の構造が良いかと思います。
理由はわかっての通り「メールアドレスの管理のみを別立てのクラスに分離にすれば、UIでメールアドレス管理しやすい」からです。

「それはちょっと手間だなぁ」と思っているようですが、実行するのはコンピュータです。
コンピュータはそんな文句は言いません。
それよりも、人が手間だと思うことを考えましょう。

人間にとって手間なのは新しいアドレス管理クラスを追加したとき変更が色んな場所に波及することです。

例えばFacebookからメールアドレスを持ってくるようなクラスを追加したとしましょう。
下記のように簡単な修正で済みますし、もともとあったMailAddressクラスを変更しなくて済みます。

FacebookMailAddress address = new FacebookMailAddress();
address.Name = address.Search("宛先名");

SendMail sendmail = new SendMail();
sendmail.Send(address, "メール本文");

このとき、FacebookMailAddressはMailAddressを継承して実装すると良いです。
なぜならこの実装だとsendmail.Send(address, "メール本文");の引数addressの型が
FacebookMailAddressに変わってしまいます。
するとやはりSendMailの中も変更しないといけなくなります。

MailAddressを継承し、メソッドをオーバーライドすることで、この問題を解決できます。
コードはきっと下記のようになるでしょう。

MailAddress address = new FacebookMailAddress();
address.Name = address.Search("宛先名");

SendMail sendmail = new SendMail();
sendmail.Send(address, "メール本文");

しかし、これではMailAddressを変更したとき、FacebookMailAddressの動作が変わってしまうかもしれません。
予期しない箇所で動作が変わることも、人間にとってはとても手間なのです。
とくに私のような人間は忘れっぽいので、変更の影響が別の場所に出ると見逃してしまいます。
そこでMailAddressの実装は最小限のものにして、もともとあったMailAddressの機能は
MailAddressを継承した別のクラスに定義しましょう。
テキトーにOriginalMailAddressという名前にします。

するとクラスの継承関係は下記みたいなカンジになります

MailAddress-OriginalMailAddress
           -FacebookMailAddress


このときMailAddressはインターフェースだけの抽象クラスとして定義することが一般的です。
例えばこんな感じ(C#書き慣れてないので間違ってたらすいません)

interface MailAddress{
    String Search(String addressName);
}

このように実装することで、沢山のうれしいことがあります。
例えば下記のこんなかんじでそれぞれのMailAddressを使い分けるようにメソッドを定義できたりします

public void sendMailOriginalMailAddress(String addressName){
  MailAddress address = new OriginalMailAddress();
  sendmail.Send(address.Search(addressName));
}

public void sendMailFromFacebook(String addressName){
  MailAddress address = new FacebookMailAddress();
  sendmail.Send(address.Search(addressName));
}

public void sendMail(MailAddress address){
  SendMail sendmail = new SendMail();
  sendmail.Send(address, "メール本文");
}

なんかキレイになったと思いませんか?

流れを追って説明しましたが、これは
「依存性逆転の原則」というものを適用した流れです。
オブジェクト指向におけるクラス設計の原則で他にあと4つあります。
それぞれの頭文字を取って「SOLID原則」と呼ばれています。

ちなみにわたしの一番好きな原則も「依存性逆転の原則」です。
「抽象に依存せよ」っていうかっこいいフレーズがたまらないんです。

では、もう一つ私の好きな原則を紹介します。
「YAGNI原則」といいます。
You ain't gonna need it.
の略だそうです。直訳すると「そんなの必要ねーよっ」って意味だそうです。
wikipediaのわかりやすい説明では「機能は実際に必要となるまでは追加しないのがよい」ってなってます。
そうなんです。ここまで長く説明したんですけど、こういう設計がもたらす便利さが不要なら、
別にそんな設計しなくてもいいんです。
そのほうがコードは短くなるし、短いコードのほうがバグは発生しにくいんです。

だから、突き詰めて考えていったときに最終的にぶち当たるのは、
クラス設計の原則を当てはめるべきかどうか?というところなんです。

クラス設計には原則のようなルールがありますが、それはあくまで道具です。
これが正しいという答えは自分で判断するしか無いんです。

オブジェクト指向とかクラス設計とかそのへんは、いろんな原則があります。
紹介したキーワードで検索すると、いろんな話が出てきます。

例えば少し難しいですが「リファクタリング:オーム社」という本があります。
家庭の医学みたいに、良くないコードの見つけ方や直し方が書いてあります。

ご参考までに

投稿

編集

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2017/08/28 08:36

    こんにちは。
    とても充実したご回答ありがとうございます。
    A-B
    -C
    のほうが構造がよいということ、なるほどと納得しました。
    たぶんそのほうがよいだろうなぁとは思っていたので、安心しました。とはいえ、最初からこのA-B/-Cという構造にはしていなかったのでクラスの作り直しはだいぶ手間ではあって、最初からこうすればよかったとか、なるほど最初に設計するのは大事だなと思ったり、最初に設計するといってもどのくらい将来の利用性や拡張性を見通すかは簡単ではないなと思ったりしています。
    拡張性を見通すといっても、ぜんぜん使いもしないものを用意していても無駄に手間ばかりかかってしまうわけで、そのあたりがまあなんというか「経験」なんだろうなぁ、と感じました。
    いただいた言葉でいえば、「YAGNI原則」ですね。

    継承は、業務で一度だけ自分で作って使ったことがあります。とてもよい機能だと思ったのですが、いっしょに仕事をしていたもうひとりの方には理解されず、なにしろ初めて使ったので利点をうまく説明できなかった記憶があります。

    MailAddressを最小にしてインターフェースだけの抽象クラスにする。インターフェースは、本で読んでこんな機能があるんだなぁ、と素通りしてそれっきりになってます。ついこのあいだWebで見たコードに実装してあったのを見て、「おおおインターフェースだ」と気づきました。本で読んだことはすっかり忘れているので、もう一度復習してみようと思います。

    オブジェクト指向におけるクラス設計の原則。
    まさにそういうのをちょうどいま勉強し始めようというところです。『リファクタリング』を早速amazonの「なか見!検索」で見てみました。歯ごたえありそうですね。Javaなので(C#とそっくりなので)読めそうです。
    ありがとうございます。

    p.s.
    Teratailのみなさまにはだいぶお世話になっていますが、Teratailの文化というのはあまり気にされなくてもよいと思います。すくなくともわたしはあまり気にしませんでした。
    もしも質問-回答になにか流儀のようなものがあるとして、わたしにとっていちばんうれしいのは、ともかく動くコードがあること、+αの知識があり向上できるきっかけになること、ですね。
    Teratailでは、動くコードをいただけることが多く、まずは困っているところを解決できるのでとても助かっています。
    今回の場合は、即解決というものではないので、そういうのとはすこし違って、受けとめたり、考えたりするきっかけになる情報が多いほうがありがたいわけです。

    キャンセル

+1

こんにちは。

リファクタリングは暫定修正が積み重なった時や当初付けていた名前が不適切になって変更した方が好ましいような時に、プログラムの見通しをよくすることを目的に行う作業と思います。

お悩みの機能は、sendmailクラスが持つ機能として宛先名で送信先を指定するべきか、メールアドレスで指定するべきかですね。現在前者の状態で後者を選択することでプログラム全体の見通しがよくなりますでしょうか?
既に把握されているようにsendmailを呼び出している箇所全てをわずかとは言えより複雑になる方向へ修正が必要ですね。そして、私にはそれにより「プログラム全体の見通しがよくなる」ようには思えないです。
しかも、同じことを何度も書くことになるのでDRY原則にも反します。

ふと、メールアドレスの管理のみを別立てのクラスに分離にすれば、UIでメールアドレス管理しやすいかもと考えたのです。

その通りですね。そのメールアドレスを管理するUIをaとした時、A-B-C/a-Cと呼び出せば良いと思います。

投稿

  • 回答の評価を上げる

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

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

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

  • 回答の評価を下げる

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

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

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

  • 2017/08/28 09:14

    こんにちは。
    おっしゃるとおり、この変更をするには、わずかではない規模かなという感じで複雑にする方向に修正が必要で、悩んでいるので相談しました。
    既存のに手を加えれば、その部分でのデバッグも必要です。
    自分用のプログラムなので困るのは自分だけですけど、とはいえかなり性根を据えて手を加える必要があり、事前にどうしたらよいかお知恵拝借、という感じでありました。
    つまり、やるのならやるけど、一本道で後戻りしにくいので、方向が合っているかどうかアドバイスがほしかった、という感じです。
    で、おっしゃるとおり、これによってプログラム全体の見通しがよくなるように、わたしもあまり思えないのです。とすると、どこかで道を間違えてしまったのか、あるいは最初から違う道を行くべきだったのか、みたいな感じで迷ってました。いまも迷っているかもしれません。
    A-B-C/a-C。
    なるほど。そのほうがシンプルですよね。そう思います。よろめきます。
    たぶん、A-B-C/a-Cとするのは、これまでに自分でも何度も書いてきたのだと思います。それで壁に当たっていると感じていて、それで相談したのだと思います。
    壁に当たっているのは、そもそも複雑な世界を写像するのに充分な複雑さをプログラムがもっていなかった場合に漏れていた情報をあらためて受けとめるのには、どうしたらよいか、みたいなところなのだと思います。うまく伝わったかどうかわからないのだけれど。
    それには、A-B-C/a-Cでは不充分で、もうちょっとなにかしなければならないんじゃないか、みたいなところなのかなと。
    とはいえ、A-B-C/a-Cで済むのなら魅力的ですよね。
    これは、DRY原則とは関係ない気がします。理解不足かな。
    DRY原則は、『リーダブルコード より良いコードを書くためのシンプルで実践的なテクニック』かなにかで読んで、重要だと考えています。極力それにしたがうようにしているというか、もう最大限そうしているつもりです。
    というか二度おなじことを書くのは、どうしても統合できない理由がある場合以外はしないです。
    ここで疑問なのは、業務用のプログラムだと、過去に書いた部分に手を加えることが禁止されることが多く、そういうところだと引数の数や型が異なるほとんど同じメソッドをもうひとつ作るとか、というようなひどいコードを書かざるを得ないことがしばしばあります。きれいなコードと動く(業務用の)コードとはぜんぜん違うんだなというなんというかしょうもない現実に直面しています。気持ちとしてはきれいなコードに強く魅かれます。
    業務用のだと、A-B/A-Cとすることは、Aに手を加えなければならないので許されず、A-B-C/a-Cなら許容範囲。場合によっては、A-B(含むC)/A-C'/a-C''みたいに作らされることもあって、げんなりします。ほとんど同じコードが200行もあったりして、それが3つも4つも(場合によっては5つも6つも)あると、それだけで1,000行とかになってたりして。
    そういうのって、みなさまどうお考えなんでしょうね?
    そういう開発現場をすくなくとも複数体験すると、きれいなコードとかうっとりするような設計なんていうのは、幻のような気持ちさえしてきます。

    キャンセル

  • 2017/08/28 11:50

    DRY原則については「この変更をするには、わずかではない規模かなという感じで複雑にする方向に修正」の話です。現状1箇所で済んでいることを多数の場所に書くということは、DRY原則的によろしくないです。

    > ここで疑問なのは、業務用のプログラムだと、過去に書いた部分に手を加えることが禁止されることが多く

    「動いているものに手を出さない」という考え方自体は「有り」と思います。また、メンテナンスしやすいようにソースを綺麗に保つのも「有り」と思います。どちらを採用するのかは、そのプロジェクト毎に異なると思います。
    技術者としては前者のプロジェクトは技術的に不毛なので可能な場合には避けたいです。なかなかそうも言ってられない場合もありますので頭痛いですが。私自身は組み込み系が主でしたので、ハードウェアそのものが変化しますから、前者は意味がなく後者のプロジェクトに所属することが多かったです。

    キャンセル

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

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

関連した質問

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