全ての処理をクラス側に任せてしまうのはよいことか

Question

###前提
こんにちは。毎度お世話になっております。友人と協力してつくっているゲームで、迷っていることがあるので質問します。なお質問には、プレイヤーを管理するPlayerクラス、マップ情報を管理するMapFieldクラスが出てきます。

RPGツクールのように、マップチップから作られたマップの上を、キャラクターが移動するゲームを作りたいです。Playerクラスの中の、move(int dx, int dy)というメソッドの内容でもめています。

###迷っていること
以下の部分のコードで友人から指摘を受けました。
現在の実装では、メインループの中で

// ****************

// dx,dy : キーボード入力
// nextX, nextY : 進みたい場所
// player : Playerクラスのインスタンス

boolean hasCollider = map.hasCollider(nextX, nextY); // マップ ( nextX, nextY ) において当たり判定があるか（通れるか通れないか）
if(!hasCollider){ // 通れたら
  player.move(dx, dy); // その方向に動く
}
// ****************

となっており、if文で通れるか通れないか判断しています。
友人の意見では、if文をとっぱらい、代わりにPlayerクラスで通れるか通れないか判断したらいいのでは？と言われました。具体的には、Playerクラスに、参照しているマップを格納するMapField型のメンバを加えます。メインループ側で、player.setMapField(map);のようにマップ情報を渡せばいいのでは？とのことです。
わたしは、現在の処理のほうがわかりやすいし、Playerクラスにメンバを増やさないのでよいと考えていますが、友人は処理はクラスに任せたほうがいいと考えています。
現在の処理と提案された処理、どちらがより良い設計でしょうか？理由も含めて回答していただけると助かります。

今回は、色々な意見が欲しいので、たくさんの方の意見が欲しいです。よろしくお願いいたします(^-^)

Answer 1

MapFieldのメンバ変数としてPlayerを持つべきでは？
例えば動く敵や動く障害物、他キャラクターなどをマップ上に置くとこれらの要素を考慮した当たり判定になるので判定処理にはたくさんの情報が必要になってきます。

とはいってもPlayerクラスをそのままではなく、MapObjectというマップ上での振る舞いをクラス化し、それを継承したPlayerMapObjectなどのクラスを作りPlayerクラスの情報を分割するといいですね。

上記を踏まえて回答すると動かすや動かせるか判断などはMapFieldとPlayerの中にあるべきではなく、現状の通りメインループの中でいいと思いますよ。
将棋でいうと駒は自分では動かないし、盤は動けるかを判断しない。駒を動かすのも、動かせるかを判断するのも盤上の情報から判断した人です。

Comments

[chankane]

敵キャラクターのような、自分以外のキャラクターのことを考えていませんでした。ありがとうございます。
私もメインループの中に書くほうが自然だとは思うのですが、下記でswordone様に指摘されて、「仕様に変更が生じた」といった状況が起こった場合に保守性の面では問題があることに気づかされました。私はゲームプログラミングを通してjavaの勉強をするのが目的なので、保守性の問題は見過ごせないと思っています。そういったことを考慮した場合でも、メインループの中に処理を書くべきかまだ迷っています。

[yona]

仕様変更は保守性ではなく、拡張性だと思いますよ。
どちらにしてもクラス分割は必須ですね。
メインループにしても判断処理などの特定の処理をクラス化すると保守性は向上します。

また、勉強というなら一度失敗してみるのが一番の経験になります。失敗したとしてもリファクタリングを勉強する良い経験になります。

[chankane]

拡張性ですね(;´･ω･)すみません。リファクタリングという単語は初めて聞きました。φ(..)メモメモ
早速クラスの分割をしたいと思います。また、上記のtacsheaven様の意見をお借りし、マップ側でmoveメソッドを作りその中でPlayerクラスのmoveメソッドを呼びたいと思います。
ありがとうございました。勝手ながらフォローさせていただきます。

Answer 2

私の意見としては友人さんに賛成です(Playerクラスに処理を持たせる)。

この移動処理が登場するのがこの1か所だけならどちらでもいいのですが、
複数の状況で移動を使う必要が出てきたら？
マップ上を移動する要因がキーボード入力以外にも必要となった際(例えば氷の上で滑る、などの状況)、
その場所すべてにこのif文処理を書くことになります。
そしてその仕様に変更が生じた際、そのすべてを修正する必要が生まれます。

そうなるくらいなら、初めから移動処理を一本化しておけば楽になると思います。
メンバが増えるのは煩雑にならない限りさほど問題とは思えません。

Comments

[chankane]

そうですか。メンバが増えるのはさほど問題ではないのですね
私はゲームプログラミングを通しての勉強が目的なので「仕様に変更が生じた場合」などの想定はありがたいです。上記のyona様のご回答の最初の部分「MapFieldのメンバ変数としてPlayerを持つべき」という部分になるほど！と思いました。私はそちらのほうが自然で、管理が楽（マップにさえプレイヤーの位置情報をわたせばよいと考えたから）だと思うのですが、swordone様はそのことについてどうお考えですか？

[swordone]

同感です。yonaさんの回答の最初の1文を読んで即座に高評価ボタンを押しました。

[chankane]

ご回答ありがとうございました。今回はyona様にベストアンサーを差し上げることにします。またお世話になると思います。

Answer 3

いわゆるウィザードリィ系のダンジョンだと、マップにいろいろな仕掛けがあります。

• 一方通行の扉
• 隠し扉
• 特定の属性でないと通れない扉
• 特定のアイテムを持っていないと通れない扉

これらに対処することを考えると、Player クラスで持っているよりは Map 側で（Player の情報を考慮したうえで）処理したほうがよさそうな気はしますね。

Comments

[chankane]

ご回答ありがとうございました。そうですね、鍵を使って扉を開けることをしようと思っているので、上記の想定に該当します。マップ側で管理するほうがどちらかというと適当だとわたしも考えています。tacsheaven様は、マップ側で処理orメインループ側で処理のどちら側がいいとお考えですか？ない場合は無理に答えなくても大丈夫です(o^－^o)

[tacsheaven]

プレーヤーが「座標Aから座標Bへ移動する」という形でマップのメソッドを呼び出す、感じですかね。（移動元と移動先がわからないと一方通行が再現できないので）
マップ側は渡された移動対象者の情報（必要なアイテムを持っているかなど）と、マップ自身の持っている情報を基に移動できるかできないかを含めて処理する、ですね。

[chankane]

ではマップ側で、メソッドmove()を呼び、その中でPlayerクラス（後でクラスを分割する予定）のmove()メソッドを呼ぶようにしたいと思います。ただ、プレーヤーが「座標Aから座標Bへ移動する」という処理は少々難しいかなぁ？と思う（最短経路がふさがれている場合回り道する等々）のでそこは別の機会にて質問します。
ご回答ありがとうございました。