ランダムジェネレータ：テストで必要なメソッドをpublicにする

[ochaochaocha3]

ランダムジェネレータのテストにおいて、Object#send を使って無理矢理プライベートメソッドを呼び出していた箇所を、普通のメソッド呼び出しに変えました。おそらくこれは私がインターフェースの設計に失敗していた箇所です（テストで使う＝外部から使う ⇒ publicでなければならない）。

ついでに、server_connection_reportのmail_generatorのテストの同様の箇所も修正しました。

[koi-chan]

本体でプライベートで良いメソッドなら、テストの時には通常とは違う方法(send)で呼び出していても良い気がしますが、そういうものでもないのでしょうか。
本番で、アダプタから直接呼び出されないメソッドがプライベートになっていた方が、呼び出すべきではないメソッドを呼んでいる(プライベートになっていてメソッドを見つけられない)エラーが発生するので、安全な設計に思えます。

[ochaochaocha3]

全部はやりすぎかもしれませんが、少なくとも load_data と logger= は send で呼び出さないほうがよいと感じました。load_data は、ちょうど外部から呼び出して環境に合わせる例になっているので、public が合う場面だと思います。logger= は attr_accessor で定義されていたので、普通に呼び出せました。

[koi-chan]

logger は元々アダプターから呼べる(厳密には書き込みさえ出来れば良いし、IRC や Discord のコマンドからは呼び出されないのでコンストラクタで書き込めれば良かったかも)ので、Object#send を使わない呼び出し方にして頂いたままで良いと思います。
無造作に send を使ってしまったのは自分のミスです。

ジェネレータのメソッドの可視性については、今まで「アダプター(もしくは他のプラグインのジェネレータ)から直接呼び出すか」だけを考えて設定してきました。
もしその基準を変えるなら、明確化してから、(このブランチでなくていいですし急ぎませんが issue に登録して)全てのプラグインのジェネレータに適用すべきではないでしょうか。

load_data に関しては、ランダムジェネレータ以外でも使う可能性が高そうなメソッドなので、public になっていても良いと思います。他のメソッドについては、現時点では単なるテストのためだけに本体を修正するだけのようですので、引き続き private のほうがいいです(これらを例えばカードデッキプラグインを実装するにあたり外から使うならその時に private を外します)。
テストはいつも作って頂いているばかりなのに注文が多くて申し訳ないのですが、よろしくご検討ください。

ついでに、先日の #158 (comment) と同様に、メソッドは Hash を返し、@table は呼び出し元で格納するように変更して頂けないでしょうか。

[ochaochaocha3]

ジェネレータのメソッドの可視性については、今まで「アダプター(もしくは他のプラグインのジェネレータ)から直接呼び出すか」だけを考えて設定してきました。

これはその通りです。理想は、外からは rg や info といったコマンドに関係するメソッドだけ public にする、ですね。改めて見直した結果、自分の書いたテストの書き方が悪かった（調べないでよいところまで調べていた）と考えました。この件については、また後でテストを書き直すことで修正したいと思います。

load_data に関しては、ランダムジェネレータ以外でも使う可能性が高そうなメソッドなので、public になっていても良いと思います。他のメソッドについては、現時点では単なるテストのためだけに本体を修正するだけのようですので、引き続き private のほうがいいです(これらを例えばカードデッキプラグインを実装するにあたり外から使うならその時に private を外します)。

分かりました。load_data だけ public にします。

ついでに、先日の #158 (comment) と同様に、メソッドは Hash を返し、@table は呼び出し元で格納するように変更して頂けないでしょうか。

一貫性がなくて申し訳ないのですが、これは迷いました。というのは、今回は呼び出し元がジェネレータ以外であり、かつ @table は外から自由な形式では変更できない方がよいからです。BCDiceのプラグインのあのメソッドはクラス内から呼び出すだけであり、またクラス内ならどこでもインスタンス変数に代入できるので、純粋な関数（値を返すだけ）にした方が名前（get_...；set_... ではない）とも合うと考えて、あのように提案いたしました。今回の load_data は、外から見ると内部の状態を変える（副作用がある）動作なので、合わないかもしれないと考えました（副作用があることを明示するために、load_data! と ! 付きで書いた方が分かりやすいかもしれません）。

[koi-chan]

返信・作業中ありがとうございます。
@table について、承知しました。このままにしておきましょう。

ほかは特にありませんので、マージお願いします。

[ochaochaocha3]

丁寧に見てくださりありがとうございました。それでは、マージいたします。