SetPosやGetPosのような今いる座標を入れたり参照したりするだけの処理文が短い関数はヘッダーに処理を書くべきか否か

Question

前提・実現したいこと

最近C++の勉強を始めて、元々C言語ベースのプログラムをC++にしようと思い、とりあえずクラスで書き直しています。
そこで、2点悩んだことがあります。

1つ目
SetPosやGetPosという関数の処理をヘッダーに書くか悩んでいます。

SetPosは各ステージごとのプレイヤーなどのキャラクターやオブジェクトの座標を決めるために使っています。
GetPosは当たり判定で使っています。

下記のコードはプレイヤーのcppに書かれたものです。

XMFLOAT3* GetPlayerPos(void)
{
	return vPos;
}

void SetPlayerPos(XMFLOAT3 pos)
{
	vPos = pos;
}

2つ目
上記のコードに書いてあるvPosは下記のようにヘッダーファイルでprivateで宣言していますが、変数名は多重定義防止のため、PlayerPosとかに変えたほうが良いでしょうか。
または、構造体化して、その中にvPosやvScl、nStateなどの情報を入れてpPlayer->vPosのように使った方が良いでしょうか。

private:
　　int		 nState;	  // 状態
	XMFLOAT3	vPos;		// 座標
	XMFLOAT3	vRot;		// 回転
	XMFLOAT3	vScl;		// 大きさ
	XMFLOAT3	vVel;		// 速度
	XMFLOAT4X4  g_mtxWorld;  // 座標変換データ

Comments

[fana]

> XMFLOAT3* GetPlayerPos(void)

質問の主題に影響はないが，戻り値の型が異なるように見える．

[.com]

ホントですね。
コピペではなく手書きで書いたので間違って書いていました。
正しくは下記です
XMFLOAT3* GetPlayerPos(void)
{
return &vPos;
}

[fana]

多くの場合，内側で抱えている物（メンバ）へのポインタ（や参照）を返すのはやめておいた方がよいです．
持ち主の知らないところで，任意のタイミングで，そのポインタを介して勝手にメンバの値を変更できる手段を提供してしまう形になります．
また，ポインタのようなものを返すと，ポインタが指す先のメンバの生存期間との兼ね合いに注意しないとバグる形になります．

「XMFLOAT3 という型のコピーのコストがかなりでかいから何とかしたいので…」みたいな理由でもない限りは，
「vPosのコピー」を返しておくのが問題が無くてよいでしょう．

[fana]

XMFLOAT3*　型で メンバvPos を指すポインタを返してしまったら，そのポインタを受け取った側が好き勝手にvPosの値を変更できるわけですから，
vPos を private にしている意味が全くなくなってしまいますね．

const XMFLOAT3 * 型ならば，その点がマシになりますが，
前述の通り，ポインタ型とする真っ当な理由がないならば，XMFLOAT3 型にするのがよいでしょう．

[.com]

色々とありがとうございます。
GetPlayerPos()に関してはXMFLOAT3の型で参照のみしたいだけです。

使う所は、当たり判定を取る時にオブジェクトの位置を参照する。と言うところくらいです。

とりあえず、提示していただいたconst XMFLOAT3 ✳︎で書いていきます。

調べた所、ポインタにも色々種類があるみたいなので少しずつ理解していこうと思います。

Answer 1

1つ目

好きにすればよい．

2つ目

何がどう「多重定義」になる恐れがあるのか？　が不明であるが，
vPos等のデータがメンバ変数である以上，その持ち主が存在するわけだから，持ち主::データ という形であるハズ．大抵はそれで十分ではないだろうか．
例えば，

class Player
{
private:
  XMFLOAT3 vPos;  //座標
};

という形であれば，このvPosは「Playerの座標」だとわかるから，わざわざPlayerPosという変数名にする必要はない．
そうではなくて，

class XXX
{
private:  //なんかいろんな奴のデータをごちゃごちゃと保持している
  XMFLOAT3 vPos;  //これは Player の座標で…
  XMFLOAT3 vPos2;  //これは他のやつの座標で…
  ...
};

みたいな形になっているとかいう話であれば，確かに名称なりデータ構造なりで分かりやすくした方が良い．

Comments

[.com]

完全に自分の勉強不足でした。

ヘッダーをインクルードした場合、他のcppファイルでも普通にvPosと置いて使えてしまうものだと思っていて、その際に例えばCollision.cppがあるとするとvPosがPlayerやBullet、Enemyなどにもあるため、そこで定義元がぐちゃぐちゃになり、バグの原因になるのかなと思っていました。

そもそもprivateにしておけば、Player::～の中でしか使えないという事、認識しました。

一つ目に関しては結構どうでも良いという事なので、cppのコード量を少なくするためにヘッダーに書きます。

かなり初歩的なことにお手数をお掛けして申し訳ございません。
ありがとうございました。

[fana]

何をどのようにうまく把握できていない感じなのかが，こちらにはいまいちわからないですが，
複数の要素が複合した塊についてなんとなく認識するのではなく，
個々の要素を１つずつしっかり把握しておいた方がよいのではないかな，とか何とか思います．

・「多重定義」　→　定義とは？　多重とは？
・「インクルード」　→　そもそもインクルードとは？
・「private」　→　どんな機能か？　{ private, public, protected }の違いは？
・etc　：　（型　と　インスタンス　の違いとか？）

[.com]

そうですね。
完全に説明しろ。と言われたら取り留めの無い事を言うと思います。

書かないと覚えられないタイプなのでコードは書きますが、座学も進めて説明できるようにしておこうと思います。

[fana]

> 1つ目

に関しては，回答が簡素すぎて投げやり感みたいなのが感じられる気もしますので，一応補足しておきます．

｛メソッドの定義を宣言部分（ヘッダ）に書くか，別のcppに定義を分けて書くか｝によって，
そのメソッドの定義を変更した際にその型の利用側（ヘッダをincludeしてる側）に再コンパイルの必要が出るか/出ないか，というところに差が生じます．
が，今回の例のような（おそらくその実装変更自体が生じることは無かろう，というほど）単純なものならば好きな形でよいかな，と．

あと，メソッドの宣言と定義を分けずに書くと，「そいつには inline を付けたことに（暗黙的に）なる」という話があります．

丁度そこらへんの話をしている質問を見つけたので提示しておきます．
https://teratail.com/questions/40481

[.com]

ご丁寧にありがとうございます。

感覚的に複雑な処理(計算とかプレイヤー操作とか)はcpp、1〜2行の処理はヘッダーという感覚であながち間違っては無いみたいで良かったです。

再コンパイルが必要か不必要かは処理時間の関係もあるので多少気にかけておこうと思います。

とりあえずSet〜とかGet〜は複雑な処理では無いのでヘッダーに書いておこうと思います。