cpp ファイルが長文になったら・・・

[Ma]

クラスの関数が何十もあり、そのひとつひとつの関数が長くなってしまった場合、
みなさんはどのように分割していますか？


１．新しく cpp ファイルをつくり半分の関数をそっちに移動する。
２．新しく h ファイルをつくり、cppファイルにインクルードすう。（外道なのだろうか？）
３．なにがあろうとcppは、クラスひとつにたいしてひとつのみ。（VC++の[+][-]のボタンで宣言内容を略で表示する。）
４．その他

よければ理由もお聞かせください。

[たかぎ]

> クラスの関数が何十もあり、そのひとつひとつの関数が長くなってしまった場合、
> みなさんはどのように分割していますか？

最初に考えないといけないのは、何十もあるというメンバ関数が、本当にメンバである必要があるのかどうかです。
非メンバ、非フレンドにできる関数はそのようにすべきです。
フリー関数になれば、3.を支持する理由は失われます。

ひとつひとつの関数が長くなったとのことですが、どの程度長くなったのでしょうか？
アルゴリズム関数を使ってループをなくすことができれば短くすることはできると思いますし、関数内の汎用的な処理は再利用可能な形に切り出すのもよいでしょう。
いずれにせよ、実際のソースを見ないことには一概にどうすればよいかはいえません。

[Justy]

　クラスのメソッドが何十もあってしかも１つ１つが長いとなると、それはもうクラス設計に
問題があります。
　そのようなクラスを放置しては将来に禍根を残すことになりかねません。

　従って「４　設計を見直す」です。


　ただ、状況的にその選択肢は無理、ということなら１か３ということになります。

　メソッドの性格・役割などの分類ができるならその分類毎に１の方法で分割します。
　分類するとその種類が多くなってしまうカオスなごった煮状態、或いは逆に１種類しか無く
同類ばかりというのであれば３です。

[Ma]

なるほどなるほど。
参考にしてみます。

＞いずれにせよ、実際のソースを見ないことには一概にどうすればよいかはいえません。
一応例に出せるようなソースをもっているのですが、あえてはらないでおきました。
理由は、ソースがめちゃ長いのと機能を説明するのが大変なのと、この場では抽象的に話すつもりだからです。
具体例があったほうが答えやすいというのはごもっともなので、ソースの代わりに簡単に言葉で説明させていただきます。（↓に関数の説明をいれました。）


＞ひとつひとつの関数が長くなったとのことですが、どの程度長くなったのでしょうか？
string を引数として、その内容次第で処理を切り替える関数が一番ながいです。
この関数は、ノベルゲームによくある読み込んだスクリプト次第で処理をかえるというのが目的です。
if else　文で分割し、そのスコープ内で、stringにあった命令文（スクリプト）どおりに動く形です。
（ココで言うスクリプトとは、私が独自のフォーマットで作ったtxtファイルから読み込み、プログラム側に指示を出す命令文のことです。）
（たとえば、文字更新、画像更新、画像移動、画像透明度変更、音声再生、ＢＧＭコントロール、などなど。）


＊私なりに思いついた解決法
この場合は、スクリプトの指示のひとつひとつをメンバー関数にして、その関数をポインタ化し、stringを引数とする例の関数から呼び出すなんて方法も思いつきます。
ですが、ヘッダーファイルがメンバー関数の数ですごいことになります。
さらに、この場合は数十あるスクリプト関数用に別にcppファイルを用意したほうがよさそうですね。




＞　従って「４　設計を見直す」です。
↑のスクリプトのような処理はやはり、クラスをわけるわけいはいかなさそうですよね。
とすると、やはりcppファイル分割がいいのかな。

[たかぎ]

> 命令文ひとつひとつをメンバー関数にして

ですから、その命令文ひとつひとつが、本当にメンバ関数である必要があるのですか？
クラスが肥大化すると、データメンバがグローバル変数と変わらなくなってしまいます。
カプセル化のやり方をしっかり見直せば、改善の余地はあるように思います。

[山崎]

私はまだひとつのcppファイルを分割しなければならないほど長くしたことはないですが、
おそらく１の方法をとると思います。
新しくヘッダを準備するのも手間ですし、
あまりに一つのファイルが長いと関数やメンバを探すのが大変ですからね…。
ええと、飽くまで私はまだまだ素人なので、参考までの意見でした。

投稿主じゃないのですが、ちょっと関連して質問してもよろしいでしょうか？

なぜ、関数を非メンバ・非フレンドにするべきなのでしょうか？

私は今まで、変数や関数は、なるべくクラスメンバやクラスメソッドにして、
なるべくグローバルは避け、スコープを狭めるようにしてきました。
メンバ関数に関しては、この努力は逆だったのでしょうか…？

[たかぎ]

> なるべくグローバルは避け、スコープを狭めるようにしてきました。

非メンバ、非フレンドにできるものをメンバ関数にすれば、クラス有効範囲（スコープ）はその分だけ広がります。
極端な話、アプリケーションクラスを作って、プログラム全体をそこに詰め込んでしまえば、アプリケーションクラスのデータメンバはグローバル変数と何ら変わらなくなりますよね。

> メンバ関数に関しては、この努力は逆だったのでしょうか…？

状況にもよりますが、基本的には逆です。目的と手段が逆転している可能性があるからです。
カプセル化ないしは情報隠蔽を考える上で大切なのは、情報の公開範囲を必要最小限に抑えることです。
情報を見せなくてもよい関数（非メンバ、非フレンドにできるはずなのにメンバにした関数）に見せてしまうのは目的に反しています。

[Ma]

＞その命令文ひとつひとつが、本当にメンバ関数である必要があるのですか？
はい、すべての関数が、クラスのメンバー変数に対する処理を行っているため非メンバーにできません。
ただ、クラスの外から参照して処理しようと思えばできる部分が多々あるので、これを非メンバーにしろということでしょうか？
たとえば、今まで話をしてきたのはスクリプトから読み込んだ文を保存しているクラスなのですが、表示する場合、メンバー関数の.draw() をやめて、外から文を保存してあるメンバーを参照して表示するなど。
ただ、この方法は、インスタンス名.メンバー変数 のような処理が多発します。
そのため、これをメンバー関数にして、インスタンス名.　の部分を略してきました。
この手法はあまり好ましくないのでしょうか？

[たかぎ]

> はい、すべての関数が、クラスのメンバー変数に対する処理を行っているため非メンバーにできません。

コード不明なので具体的な話はできませんが、個々のデータメンバに対して（他のメンバ関数ではできない方法で）デリケートな操作を行う必要があるなら仕方がありませんね。

> ただ、この方法は、インスタンス名.メンバー変数 のような処理が多発します。
> そのため、これをメンバー関数にして、インスタンス名.　の部分を略してきました。

つまり、データメンバがグローバル変数に近い状況になっているわけですね？

> この手法はあまり好ましくないのでしょうか？

一般論では好ましくありません。

[Ma]

なるほど、みなさんのおかげでよく分かりました。
クラスを間違った方向に充実させすぎると、今度はグローバル化していってしまうため、
本来のクラスの目的が無意味になっていってしまうんですね。
自分のオブジェクト指向も、今後はより良い設計がされたものを作れると思います。
ありがとうございました。

[Justy]

　なんか解決したようですが、一応コメントしておきます。
　
　実際問題こちら側はソースがないので、あくまで一般論というかこちらが想定している状況でしか
言えないので、あちこちに齟齬がありそうですが。


＞string を引数として、その内容次第で処理を切り替える関数が一番ながいです
　それなら例えば std::mapのキーに文字列、値に処理する関数ポインタなり関数オブジェクトなどを入れておいて、
検索・切り替え処理をすればコードは一気に短くなりそうですね。


＞↑のスクリプトのような処理はやはり、クラスをわけるわけいはいかなさそうですよね
　この手のゲームスクリプトの性格上、スクリプトからゲーム全体のフラグ制御をする必要もあるので
グローバル的なデータな部分があるのも解らなくはないです。

　ただ、今の設計のスクリプトクラスは処理するスクリプトが増える度に肥大化していくのが
目に見えているので、上手くこのクラスから分離して処理するのがいいかと思います。

[Ma]

>　それなら例えば std::mapのキーに文字列、値に処理する関数ポインタなり関数オブジェクトなどを入れておいて、
>検索・切り替え処理をすればコードは一気に短くなりそうですね。

あ、それに似た方法はすでに思いついています。(mapとまでは言いませんでしたが)
　　　ここで↓
　＞＊私なりに思いついた解決法

その関数ポインタの関数をどこに配置するかというと、結局その関数はクラスのメンバー関数になりますよね。
（その関数が、クラスのメンバー変数にたいして処理を行うため。）



＞　ただ、今の設計のスクリプトクラスは処理するスクリプトが増える度に肥大化していくのが
＞目に見えているので、上手くこのクラスから分離して処理するのがいいかと思います。

一応、一定量のスクリプトに達したらデータを捨てる方法をとって膨大化をふせいでいました。


解決おしわすれてました。
どのようにして回避するべきか、またどうしても長くなってしまった場合、どの状況に応じてどの手法をとるべきかという基準が大体分かったので解決にします。
（一応また書き込みがありましたら見ます。）

[山崎]

たかぎさん
ご回答いただきありがとうございます。
確かに手段と目的が逆転しているような気がします、
なるべくカプセル化をするにはどうすればいいのだろう、と
その部分に苦心してしまっていた傾向がありました。
非常に参考になりました、ありがとうございました。

Maさん
途中で質問を挟んでしまい申し訳ございませんでした。

[バグ]

既に皆さんが書かれているように設計に難有りというのが私の意見ですが、それだけではなんなので…

VisualStudio限定ではありますが、#pragma region、#pragma endregionで関連あるメソッドを折り畳む…というのはいかがでしょうか？
根本的な解決になっていませんが、とりあえず、見やすくなるかと思いますよ。

[Justy]

＞結局その関数はクラスのメンバー関数になりますよね
　どうでしょうね。

　例えば汎用のシステムとして考えるなら処理の項目がべったりクラスにひっついていると、
それはもう汎用ではなくなりますので、項目の種類を外部から定義したり、
諸々の処理を外部で処理できるように作ります。
　その場合その共通でアクセスするデータも外部におかれるので、もうメンバ関数にはなりません。
（仮にその際それがメンバ関数になることはあったしてもそれは別のクラスのメンバ関数でしょう）

　反対に汎用にはせず、べったりそのクラスに項目を固定してしまうのであれば、
メンバ関数になることもあります。

　設計と状況次第ですね。


＞＞肥大化していくのが目に見えているので　
＞データを捨てる方法をとって膨大化をふせいでいました
　あ、いえ、肥大化していくのはデータではなくクラスの規模の方です。

[Ma]

>途中で質問を挟んでしまい申し訳ございませんでした。
いえいえ、かまいませんよ。

>、#pragma region、#pragma endregion
おぉ、故意的に範囲を指定できたんですね。
勉強になりました。ありがとうございます。


>　設計と状況次第ですね。
なるほど。たしかに非クラス関数としてもよさそうですね。
そうなると、cppファイルを新しく作りそこに関数郡をおいて、そのポインタを使うことで整理できそうですね。
例のクラスのメンバーへのアクセスには、
インスタンス名.メンバー名　となりますね。
（追記：インスタンス名はわからないのでポインター化した関数の引数に対象のインスタンスのポインターか参照でも渡せばいいのかな。）

>諸々の処理を外部で処理できるように作ります。
メンバーをパブリックにするってことですね。
貴重なご意見ありがとうございました。


>あ、いえ、肥大化していくのはデータではなくクラスの規模の方です
あぁ、勘違いしてたみたいですね。すいません。

[たかぎ]

なんか、全然伝わっていないような...
やはり、実際のコードを見ながら、具体的な話をした方がよいかと思います。

[Ma]

自分が間違って読んでいるかもしれませんよ？
ということなので、今回聞いたことの一部を模擬コード化しました。
元のコードをそのままのせると、とんでもないことになるので、模擬コードを変わりに作りました。
以下、模擬コードです。

とおもったら、長すぎてはれなかったので、添付しました。

間違って理解しているようであればご指摘おねがいします。

かなり眠い状態で作ったので、模擬コードでも間違いがあるかもです。
模擬コードでこんだけ長いんだから、本物のせたら大変なことに。。。
（すいません、これから時間的にしばらく（半日？）返事できません。）

[たかぎ]

> 模擬コードでこんだけ長いんだから、本物のせたら大変なことに。。。

全然長くありませんので、本物をのせてください。
少なくとも伝わっていないことだけは確認できました。

[Poco]

とりあえず、「オブジェクト指向」と「SRP（Single Responsibility Principle）」で
ググッてみてはどうでしょうか？

クラスが大きくなったから、ファイル分割を行うのは何の意味もありません。

[Ma]

＞全然長くありませんので、本物をのせてください。
＞少なくとも伝わっていないことだけは確認できました。

そうですか。。理解していなかったようですいません。

では、仕方ないので本物をそのままはることにします。
たぶん、読むだけで数時間かかるかも。。。本当にすいません。

(このコードを修正するのはお金とれるような読み量/修正量だとおもうので
もし、長すぎて気分を害されたようでしたら、本当に申し訳ないです。
量がきついようでしたら見ていただけなくても結構ですので…。それでも見ていただけるようなら、本当に感謝です。）

あと、生半可な知識で無理やりごりごり完成させたゲームのソースなので、非効率的なところや、
不可解な部分が多数あるとおもいますが、ご了承ください。

コードは、私が製作したゲーム「シエル・ソルシエールズ」のコードのものです。
一応バグはなく（プロジェクトとしては）コンパイルできるものですが、添付したファイルだけではコンパイルできません。

よろしくおねがいします。

[Ma]

>Title: Re:cpp ファイルが長文になったら・・・
> とりあえず、「オブジェクト指向」と「SRP（Single Responsibility Principle）」で
>ググッてみてはどうでしょうか？
入門時代に、（もう覚えていない）あるサイトで一度読んだきりでしたので、
再度読んでみることにしてみます。

[たかぎ]

この程度であれば全然長くありません。
ただ、もうこの時間ですし、週末も終わりましたので、いつ時間が取れるかわからなくなりました。
それと、storyStructure.cpp が空のようです。

[Poco]

>たぶん、読むだけで数時間かかるかも。。。本当にすいません。
おそらく大抵の方は、*.cppの方は行数確認するだけなので問題ないと思います。

軽くヘッダファイルを見ましたが、storyControllerクラスの役割多くないですか？
Maさんも「（たとえば、文字更新、画像更新、画像移動、画像透明度変更、音声再生、
ＢＧＭコントロール、などなど。）」と仰ているように、 このクラス単体で実現
しようとしていることが多すぎる気がします。

私なら、DisplayManager（文字更新、画像更新、画像移動、画像透明度変更担当）、
VolumeManager（音声再生担当）、BgmManager（ＢＧＭコントロール担当）といった
クラスを作成し、storyControllerクラスはシナリオ解釈後、やるべき事に対して、
それらのクラスを呼び出すだけの実装にします。

というわけでこの場合なら、私もJustyさんと同様、
4.設計を見直す。
です。

[Ma]

>それと、storyStructure.cpp が空のようです。
すいません。 story_structures.cpp を作る前に誤って作った物です。
まったく関係ありませんので、けしていただいてかまいません。





オブジェクト指向について読み直せ　という指摘から、ちょっと復習させていただきました。
（一応、私なりのオブジェクト指向の理解度を示すためにいいと思ったので書かせて頂きました。）

@IT 連載 ここから始まるオブジェクト指向 を読んで、私なりに気づいた現在の問題点

●自律分散協調動作
○いろいろつめこみすぎて、グローバル化している。
役割が拡散していない。

●オブジェクトは外部からのメッセージを受け取るためのインターフェイスを公開しています。メッセージを受け取ると、それに対応する振る舞いを実行するのですが、外部からは、公開されているインターフェイス以外にオブジェクトの状態に直接アクセスする方法はありません。
○外部からコントロールするために、set()系の関数を作るのが面倒で
メンバーをすべてpublic にしちゃってる。


●（3）関係
　オブジェクトは単独で存在することはありません。必ずほかのオブジェクトと関係を持っています。
○story_structures と storyController があるが、ふたつだけではなく
もっと関係を増やし、分解していくべき？

●（4）アイデンティティ
各オブジェクトは、ほかのオブジェクトから自らを識別できるアイデンティティを持っています。
○ぜんぜんない！というか、いくつも作る目的で作ったクラスではなかったため、アイデンティティは必要でなかった。
↑すでに、オブジェクト指向ではないことが伺える…



○私のソースについて考えると

つまり、storyController はクラスの長所をぜんぜん使っていないのだから、そもそもクラスであるべき必要はないしオブジェクト指向ではない。
クラス（または構造体）使っているんだから、一応オブジェクト指向だと思ったら大間違いということなんですね。

それに対して、lineInfo や　chara とかの構造体は、複数作るための構造体なのでオブジェクト指向として機能しているかのように見えますが、スーパークラスがない点などからクラス階層を欠如しているためオブジェクト指向ではない。ということでしょうか。

さらにポリモフィズムなんて、ぜんぜん使っていないので、わたしが添付したソースには
まったくオブジェクト指向があるとはいえない。

あれ、では、クラスというものはオブジェクト指向でないと使ってはいけないものなのか？
たしかに、単純にデータをグループ化したいという意図で作っていたのでは、間違い。（storyController のように）

ただし、例外として、 ココで紹介されているシューティングの敵の構造体などは階層関係やポリモフィズムをもっていなくとも、グループ化することで扱いやすくなるので、クラスや構造体を使うのが適している。
（storyController は例外ではなくクラスを使うべきではないが、lineInfo や chara は例外でクラスや構造体を使うのが良い）



と、私は読んで思いました。

いままで、オブジェクト指向だと思い込んでいたものは、いろいろと問題があったんですね・・・。
読んだおかげで、皆さんがいっていることの理解度がすこしふえたきがします。

[softya]

クラスに関しては独学で学んだのでアレコレ語れるほどの技量は有していませんが、ざっとみて機能の詰めすぎ込と言うか本来クラス分けすべき部分がひとつのクラスに詰め込まれている印象です。
シナリオスクリプの制御すべきクラスが、メニューの制御やら表示の制御まで、果ては「マウスがボタン上に乗っているかどうか。」とかsave/loadでファイルの入出力まで管理しています。
１つのクラスの保持する情報と機能がシンプルに整理されているか見直すことをオススメします。と言う事で私も

4.設計を見直す。

です。

[Ma]

なるほど。
ぽこさん、softyaさん、お返事ありがとうございます。


４．役割分割して、階層関係を作る。
これが一番ですね。ありがとうございました。

>たかぎさん
>この程度であれば全然長くありません。
>ただ、もうこの時間ですし、週末も終わりましたので、いつ時間が取れるかわからなくなりました。

お時間をとってしまいすいません。
ですが、具体的な修正案は提示していただけなくても結構です。
今回のソースは開発がほぼ終了しているものだからと、今回質問をさせていただいたのは、今後のプログラミングに役立たせるために聞いたものでした。

役割をつめこめすぎ、という皆様のご意見のおかげで、このケースの場合は具体的に何をするべきなのかは自分で探せると思います。
お手伝いありがとうございました。

[Ma]

先ほどの読書感想文みたいなものに追記です。
>（storyController は例外ではなくクラスを使うべきではないが、lineInfo や chara は例外でクラスや構造体を使うのが良い）
とありましたが、
「もしくは、storyController 内部の機能を分割し、オブジェクト指向にする。」
ですね。
それで、これが今回の解決法でした。

[たかぎ]

> ですが、具体的な修正案は提示していただけなくても結構です。

了解しました。
他のみなさんと同意見ですので、ほぼそれで解決するでしょう。

[softya]

>それに対して、lineInfo や　chara とかの構造体は、複数作るための構造体なのでオブジェクト指向として機能しているかのように見えますが、スーパークラスがない点などからクラス階層を欠如しているためオブジェクト指向ではない。ということでしょうか。

クラスだから、スーパークラスが必要だってのは間違いです。

>さらにポリモフィズムなんて、ぜんぜん使っていないので、わたしが添付したソースにはまったくオブジェクト指向があるとはいえない。

ポリモフィズムもクラスの必要条件ではありません。
オブジェクト指向の三大要素であるカプセル化、継承、ポリモフィズムを全て満たしているクラスを作らなければならないってのはオブジェクト指向の曲解だと思いますよ。

>あれ、では、クラスというものはオブジェクト指向でないと使ってはいけないものなのか？

使ってはいけないとは言いませんが、Ｃの関数で事足りるなら構造化プログラミングのモジュールと言う考え方だけで機能的でメンテンナンス性の高いプログラムを作ることは出来ます。
http://www2.cc.niigata-u.ac.jp/~takeuch ... cture.html
今のソースコードを見る限り、構造化プログラミングの点でも不十分です。今のコードはＣに無い機能を使いためだけのベターＣとして使われていると思います。

>たしかに、単純にデータをグループ化したいという意図で作っていたのでは、間違い。（storyController のように）

今のコードはカプセル化を満たしていませんので、オブジェクト指向の三大要素の全てを持っていませんよね。なのでクラス化する意味がないって事は確かだと思います。

[たかぎ]

ちなみにC++では、構造体や共用体もクラスの一種です。
ですので、オブジェクト指向ではなくても、クラスを一切使わないというのは現実的ではありません。

[Ma]

前話したように、オブジェクト指向でない場合でも、グループ化としてクラスや構造体が有効になるときがある。ということですよね。
（特に、そのグループ化したものをたくさん複製する場合）

図示すると、

オブジェクト指向
　　　↓
クラスや構造体を使用する。

ではなく、

クラスや構造体の用途
　　　↓
グループ化または、オブジェクト指向。

です（よ）ね。