sapporocpp / othello-battler

MIT License
0 stars 0 forks source link

参照による値の受け戻しをreturnへ仕様変更する #33

Open usagi opened 8 years ago

usagi commented 8 years ago

例えばこういうの

https://github.com/sapporocpp/othello-battler/blob/master/othello_runner.cpp#L26

引数で参照を受けて関数内で結果をそれに返すという実装、これを例えばtupleで必要な情報をまとめて返すなどに変更する。

参照で受けて使う関数は呼び出し前に変数を用意する必要があり、小さな事でも初心者には手間やわかりにくさを生じうる。実行速度要求があるわけでもない状況では return に tuple などで「結果」はまとめてしまうのが関数のin/outとしても、C++レベルのコードとしてもわかりやすくなり妥当と思われる。

usagi commented 8 years ago

25 などにも関連したことで、現状ではシグニチャーから何がINで何がOUTかわからないということもコードリーディング上のコストを伴ってよくない。

ignisan commented 8 years ago

この場合はtrue/falseを返しているので、返り値のみで情報を返そうとするとoptionalが欲しいところだけどoptionalは1yなので使えない。そこどうしたいのかコメントが欲しい。

usagi commented 8 years ago

もっとも初心者にもわかりやすいと思われる方法は座標を表す型を用意して、その型に is_valid という「扱い可能な正常な位置」を保持しているかを判定するメンバー関数を用意しておく。

struct board_position_type final
{
  std::int_fast16_t x = 0, y = 0;
  auto is_valid() const noexcept { return x != -1 and y != -1; }
};

これを用意するとプレイヤーは rc などとするよりも座標を扱うのだというわかりやすさがオブジェクティブな思考の有無によらず捉えやすくなる副作用もある。

これを採用すると件の関数のシグニチャーは

auto parse_sent_string( const char* buf, const std::string& nonce ) -> board_position_type;

となり飛躍的にわかりやすさが向上する。

他の方法もあるが、初心者にもわかりやすく全体のコードも可読性を向上せしめうるこの解決方法が今回ケースには妥当と思われる。


するのだが、別件として、 const char* bufconst std::string& nonce 、そしてこの関数自体が懸念する初心者にはなんの関数だかわからないし何の引数だかわからないしどう使うのかもわからないと思うので、それはそれで再設計を要すると思われる。

ignisan commented 8 years ago

私も新しく構造体を作るのが簡単かと思う。しかしそれならば、

bool is_valid(board_position_type pos) { ... }

で良く、メンバ関数である必要はなさそう。

maraigue commented 8 years ago

ここについては、「内部的な処理であって、構造体をわざわざ作るほうが煩わしい」という理由から、複数引数の参照渡しとしていました。外部に公開するAPIとするならば構造体ないしクラスを作っていたと思います。