関数の参照戻り値はconst参照にすべきなのか

Question

C++17環境で開発しています。
コードレビューで、自前クラスを参照として返している関数において、戻り値をconst参照にした方がよいのでは？と指摘を受けました。
調査をしたところ、戻り値の内容を修正している箇所があり、単純にconst参照に変更することはできないことがわかりました。

この場合、リファクタリングとしてconst参照で成り立つように設計を変えた方がベターなのでしょうか（変更コストについては考慮しなくてよいです。設計上どちらがよいか伺いたいです）

また、もし、const参照にする場合は、変更したいパラメーターに対して、セッターを新設して修正処理を切り分ける形が思いつきますが、この形でよいのでしょうか。

Answer 1

何のために参照を返り値としているのでしょうか？

• その参照を通じて何かを書き換える目的であれば、「const参照に置き換える」という手段は取れません。たとえば、std::vectorのatやoperator []は、元の配列を書き換えられる、非constな参照を返すバージョンがあります。
• コピーコストを削減したい場合は、構造を変えることで対応可能かもしれません。

Comments

[tappe]

戻り値を書き換えるために非constの参照を返しています。
std::vectorなどにも、非constの参照を返すバージョンがあるのであれば、必ずしも非const参照を返すことが、設計上よくないということではないということだと思いますので、レビュワーに値を修正している箇所があるので、非constのままとしたい。という形で返答してみようと思います。

Answer 2

変更コストについては考慮しなくてよいです。設計上どちらがよいか

という話であれば，

調査をしたところ、戻り値の内容を修正している箇所があり、単純にconst参照に変更することはできないことがわかりました。

という現状の用いられ方とは無関係に考えるべき事柄でしょう．

• 「既に戻り値を介していじくっている場所があるから」→「そのままの形で」　というのは変更コストを考えて手を付けないという方向性の話．
• 「そもそもそのような意図なのだから」→「この形が正当なので」　というのが，「設計上…」という方向性の話かと．

後者側を考えたいのであれば…

constでない参照を返すというのは，当然ながら「その参照を介していじくられることを想定した記述」なのであって，あなたがその形が適切だと考えてコードを書いたのであれば，その意図をそのまま説明すればよいだけと思います．

別の回答のコメント欄にて

いわゆるゲッターを想定しており

とされていますが，その「想定」が「本当に非constを想定したものかどうか」というだけの話ですね．
（その「想定」自体が良いか悪いか，については，レビュー者と話し合ってください，としか．）

Comments

[tappe]

レビュワーと話し合った結果、非constのままで問題ないという形になりました。
アドバイスありがとうございました。

Answer 3

そのコードレビューは妥当ですか？

「const化できないものをconst化せよ」という要望に見えます。もちろん、他にも方法はあるでしょうが、代替えが理に適うかは題意からは読み取れません。

Comments

[tappe]

コードレビューの意図は、参照を返している関数名が、getHogeといった、いわゆるゲッターを想定しており、「ゲッターで返したものは必要でない限り、const化して変更できないようにしていたほうがよいのではないか」ということだと思います。
ただ、今回は戻り値を変更している箇所が見つかったので、constを付与しないこのままの形がよいのか、別の良い修正方法があるのかと思った次第です。

[maisumakun]

> 参照を返している関数名が、getHogeといった、いわゆるゲッターを想定しており

「名前を変える」というのも立派な解決法かと思います。