概要
前々稿のコメントで不具合の指摘を貰い、前稿で不具合を再現したダイスの実装について、コードを改修して再テストするよ。
【前々稿】
【前稿】
改修
不具合の改修
↓改修が必要なDice.pyのコード。
'''
Created on 2022/10/08
@author: 芸夢 作郎
'''
import random
import time
class Dice():
# TODO インスタンス変数を使用するよう改修
seedValue = None
def __init__(self, aSeedValue = None):
if (aSeedValue == None):
self.seedValue = int(time.time() * 1000)
else:
self.seedValue = aSeedValue
random.seed(self.seedValue)
return
# TODO 最大面数（aMaxSidesNum）に整数値以外が指定された場合の対処法も考慮
def roll(self, aMaxSidesNum):
ret = random.randint(1, aMaxSidesNum)
return ret
まず、クラスの定義直下の変数の宣言はクラス変数扱いになる。
# TODO インスタンス変数を使用するよう改修
seedValue = None
なので↑は削除。
削除するだけでよくて、インスタンス変数を宣言し直す必要はないらしい。
self.seedValue = int(time.time() * 1000)
上記の記述だけで、seedValueインスタンス変数が暗黙的に宣言されて値が代入されて使用可能になるってことね。
コードを眺めたときにどんなインスタンス変数を使用しているのか一目瞭然じゃないのが気持ち悪いけど、そのうち慣れるものなのかな？
さて、__init__関数やroll関数内で使用しているrandint関数の呼び出しも問題あり。
っていうかこっちのほうがseedValueクラス変数よりも問題。
どうやらrandomモジュールはインスタンス化可能らしい。
Diceクラスをインスタンス化して複数同時に使用するなら、インスタンスが包含するrandomも、付随して個別のインスタンスを持つべきだよね。
というわけでインスタンス化したrandomをどうDice内で保持／使用するか。
以下の2つの方法が可能。
方法1
# コメントで教えてもらった方式（関数ポインタ？）
self.randint = random.Random(self.seedValue).randint
# ・・・
# ダイスロール時
ret = self.randint(1, aMaxSidesNum)
方法2
# randomインスタンスを保持
self.random = random.Random(self.seedValue)
# ・・・
# ダイスロール時
ret = self.random.randint(1, aMaxSidesNum)
方法2ならrandomモジュールの他の機能が必要になった時に呼び出すことができるけど、Diceクラスに求められる役割って、1以上の自然数を返すことだけだから、randintが呼び出せれば十分だしなー。
というわけで方法1を採用。機能の拡張が必要になったら直せばいいし。
randomモジュールをインスタンス変数で持つと、外部から覗かれてシード値勝手に書き換えられるリスクがあるんじゃないか？？って書いてるうちに思った。
というわけで方法1一択。
あと、シード値を現状インスタンス変数として保持しているけど、不要な気がしてきた。
使用中にシード値を覗かれるリスクがあるし（気が向いたらそのうち検証するかも知れないししないかも知れない）。
Pythonのクラス／オブジェクトってメンバ変数を完全に隠ぺいするような仕組みはないっぽいね。
以下参考にしたサイト。
テスト時に確認が必要だと思って保持してたけど、自分でシード値を制御してるんだから判るよね。
というわけで、シード値をインスタンス変数から除外。
コーディング規約にプチ準拠
不具合とは関係ないけどコメント指摘でもらった通り、Pythonには↓のようなコーディング規約があるらしいね。
コメントで何度も言ってる通りコーディング規約に積極的に従う予定はないけど良いものは取り入れたい。
というわけで、以下のルールには準拠しようと思う。
None のようなシングルトンと比較をする場合は、常に is か is not を使うべきです。絶対に等値演算子を使わないでください。
Javaでいう等価演算子「==」とequals()メソッドの関係性なのかな？
もしそうだとすればコーディング規約のレベルを超えた話なんだけど。。
いずれにせよコードをパッと読んだ時に何を比較しているのかはっきり判るのは良いよね。
そんなこんなで、最終的に以下のコードになった。
'''
Created on 2022/10/08
Modified on 2022/10/10
@author: 芸夢 作郎
'''
import random
import time
class Dice():
def __init__(self, aSeedValue = None):
if (aSeedValue is None):
_seedValue = int(time.time() * 1000)
else:
_seedValue = aSeedValue
self.randint = random.Random(_seedValue).randint
return
# TODO 最大面数（aMaxSidesNum）に自然数以外の値が指定された場合の対処法も考慮
def roll(self, aMaxSidesNum):
ret = self.randint(1, aMaxSidesNum)
return ret
下の方にTODOが残ってるね。いずれどこかで対応するよ。
再テスト
さて改修したコードを再テスト。
シード値を保持するクラス変数を除去したので、それに合わせてテストコードも以下のように改修。
printの文言に出力するシード値をリテラルに修正しただけだよ。
'''
Created on 2022/10/09
@author: 芸夢 作郎
'''
import time
from Dice import Dice
def test1():
print('テスト1実行開始')
# シード値に350を指定する
diceExplicit1 = Dice(350)
print('① シード値指定1回目／シード値: [350]')
for i in range(0, 100, 1):
# 1000面ダイスを100回転がす
print(diceExplicit1.roll(1000))
print()
time.sleep(2)
# シード値を指定しない
diceTimestamp1 = Dice()
print('② シード値にタイムスタンプを使用1回目／シード値: [生成時のタイムスタンプ]')
for i in range(0, 100, 1):
# 1000面ダイスを100回転がす
print(diceTimestamp1.roll(1000))
print()
time.sleep(2)
# シード値に350を指定する
diceExplicit2 = Dice(350)
print('③ シード値指定2回目／シード値: [350]')
for i in range(0, 100, 1):
# 1000面ダイスを100回転がす
print(diceExplicit2.roll(1000))
print()
time.sleep(2)
# シード値を指定しない
diceTimestamp2 = Dice()
print('④ シード値にタイムスタンプを使用2回目／シード値: [生成時のタイムスタンプ]')
for i in range(0, 100, 1):
# 1000面ダイスを100回転がす
print(diceTimestamp2.roll(1000))
print()
time.sleep(2)
# シード値に350を指定する
diceExplicitTheOtherSide = Dice(350)
print('⑤ シード値指定3回目（面数を変更）／シード値: [350]')
for i in range(0, 100, 1):
# 2000面ダイスを100回転がす
print(diceExplicitTheOtherSide.roll(2000))
print()
time.sleep(2)
# シード値に700を指定する
diceSeedVal700 = Dice(700)
print('⑥ シード値700／シード値: [700]')
for i in range(0, 100, 1):
# 1000面ダイスを100回転がす
print(diceSeedVal700.roll(1000))
print()
return
def test2():
print('テスト2実行開始')
# シード値に350を指定する
dice350A = Dice(350)
time.sleep(2)
# シード値を指定しない
diceTimestamp1 = Dice()
time.sleep(2)
# シード値に350を指定する
dice350B = Dice(350)
time.sleep(2)
# シード値を指定しない
diceTimestamp2 = Dice()
time.sleep(2)
# シード値に700を指定する
dice700 = Dice(700)
for i in range(0, 100, 1):
# 1000面ダイスを100回転がす
print('ロール{}回目'.format(i + 1))
print('シード値350のダイスロール1／出目: [{}]'.format(dice350A.roll(1000)))
print('シード値350のダイスロール2／出目: [{}]'.format(dice350B.roll(1000)))
print('シード値にタイムスタンプを使用したダイスロール1／出目: [{}]'.format(diceTimestamp1.roll(1000)))
print('シード値にタイムスタンプを使用したダイスロール2／出目: [{}]'.format(diceTimestamp2.roll(1000)))
print('シード値700のダイスロール／出目: [{}]'.format(dice700.roll(1000)))
print()
print()
return
if __name__ == "__main__":
test1()
test2()
実行結果を抜粋すると以下の通り。
テスト1実行開始
① シード値指定1回目／シード値: [350]
912
372
822
185
412
（中略）
304
528
982
611
615
（中略）
③ シード値指定2回目／シード値: [350]
912
372
822
185
412
（中略）
304
528
982
611
615
（中略）
⑥ シード値700／シード値: [700]
171
925
197
459
717
（中略）
154
428
988
444
845
テスト2実行開始
ロール1回目
シード値350のダイスロール1／出目: [912]
シード値350のダイスロール2／出目: [912]
（中略）
シード値700のダイスロール／出目: [171]
ロール2回目
シード値350のダイスロール1／出目: [372]
シード値350のダイスロール2／出目: [372]
（中略）
シード値700のダイスロール／出目: [925]
ロール3回目
シード値350のダイスロール1／出目: [822]
シード値350のダイスロール2／出目: [822]
（中略）
シード値700のダイスロール／出目: [197]
ロール4回目
シード値350のダイスロール1／出目: [185]
シード値350のダイスロール2／出目: [185]
（中略）
シード値700のダイスロール／出目: [459]
ロール5回目
シード値350のダイスロール1／出目: [412]
シード値350のダイスロール2／出目: [412]
（中略）
シード値700のダイスロール／出目: [717]
（中略）
ロール96回目
シード値350のダイスロール1／出目: [304]
シード値350のダイスロール2／出目: [304]
（中略）
シード値700のダイスロール／出目: [154]
ロール97回目
シード値350のダイスロール1／出目: [528]
シード値350のダイスロール2／出目: [528]
（中略）
シード値700のダイスロール／出目: [428]
ロール98回目
シード値350のダイスロール1／出目: [982]
シード値350のダイスロール2／出目: [982]
（中略）
シード値700のダイスロール／出目: [988]
ロール99回目
シード値350のダイスロール1／出目: [611]
シード値350のダイスロール2／出目: [611]
（中略）
シード値700のダイスロール／出目: [444]
ロール100回目
シード値350のダイスロール1／出目: [615]
シード値350のダイスロール2／出目: [615]
（中略）
シード値700のダイスロール／出目: [845]
シード値を固定（350、700）したダイスの出目の順番が、テスト1とテスト2で一致していることが確認できる。
というわけで改修完了。
たかがダイスされどダイス。
色々覚えられて勉強になったよ。
ではまた。