今日も見に来てくださって、ありがとうございます。石川さんです。
さて、その３ですが、その１を書いたときに宣言した通り、リファクタリングをやります。pylintさんから、こんなリファクタリングをした方がいいんじゃない？、と、提案がありました。
************* Module connecter_test2 R0902: 11,0: Entity: Too many instance attributes (9/7) R0913: 13,4: Entity.__init__: Too many arguments (6/5) ------------------------------------------------------------------ Your code has been rated at 9.75/10 (previous run: 9.75/10, +0.00)
はい、Entityのインスタンスのアトリビュートが多すぎますよ、初期化のメソッドの引数が多すぎますよ、と、申しております。インスタンスのアトリビュートは上限7個が適当だとpylint設計者は考えているようですね。確かに、TMで分析してアトリビュートはだいたい5個未満にならないとおかしいもんね。たまにアトリビュートが多いヤツもありますけどねぇ。7個までというのは、直観的にもいい数字ですね。そして、引数の上限は5個、まあ、妥当でしょうね。あんまり引数が多すぎると、間違いのもとですからね。
アトリビュートを見てみますと、canvas、x、y、width、height、id、start_x、start_y、connectionsと、、、まあまあ多いですね。Pointをつくるとx、yは一つになりますね。どうしましょうか。今回は箱をつくるということで、Rectangleを用意してみることにしました。そして、前からずっと使いたかった、namedtupleを使ってみることにしました。
ソースコード
ソースコードは以下の通りです。
import tkinter as tk
from collections import namedtuple
Rectangle = namedtuple('Rectangle', ['x', 'y', 'width', 'height'],
defaults=(60, 40))
class Entity():
''' Entity class '''
def __init__(self, canvas, rectangle):
self.canvas = canvas
self.rectangle = rectangle
self.start_x = self.start_y = None
self.connections = []
self.id = self.canvas.create_rectangle(rectangle.x, rectangle.y,
rectangle.x + rectangle.width,
rectangle.y + rectangle.height,
fill="lightblue", width=3)
self.canvas.tag_bind(self.id, "<ButtonPress>", self.button_press)
self.canvas.tag_bind(self.id, "<Motion>", self.move)
self.canvas.tag_bind(self.id, "<ButtonRelease>", self.button_release)
def button_press(self, event):
''' マウスのボタンが押されたときの処理 '''
self.start_x = self.canvas.canvasx(event.x)
self.start_y = self.canvas.canvasy(event.y)
def move(self, event):
''' マウスが移動したときの処理 '''
if event.state & 256: # マウスボタン1が押されているときだけ（ドラッグ中のみ）
can_x = self.canvas.canvasx(event.x)
can_y = self.canvas.canvasy(event.y)
coords = self.canvas.coords(self.id)
coords[0] -= self.start_x - can_x
coords[1] -= self.start_y - can_y
coords[2] -= self.start_x - can_x
coords[3] -= self.start_y - can_y
self.canvas.coords(self.id, coords)
self.start_x = can_x
self.start_y = can_y
self.rectangle = Rectangle(*coords[0:2], *self.rectangle[2:4])
for connection in self.connections:
connection.move(self)
def button_release(self, event): # pylint: disable=unused-argument
''' マウスのボタンが離されたとき '''
self.start_x = self.start_y = None
def get_center(self):
''' 中心座標を戻します '''
center_x = self.rectangle.x + self.rectangle.width//2
center_y = self.rectangle.y + self.rectangle.height//2
return center_x, center_y
def add_listener(self, connection):
''' コネクションのリスナーを登録します '''
self.connections.append(connection)
class Connection():
''' Connection class '''
def __init__(self, canvas, start_entity, end_entity):
self.canvas = canvas
self.start_e = start_entity
self.end_e = end_entity
start_entity.add_listener(self)
end_entity.add_listener(self)
self.id = self.canvas.create_line(self.get_intersection(start_entity),
self.get_intersection(end_entity))
def move(self, entity):
''' エンティティが移動したときの処理（エンティティから呼び出される）'''
coords = self.canvas.coords(self.id)
if entity == self.start_e:
coords[0:2] = self.get_intersection(entity)
coords[2:4] = self.get_intersection(self.end_e)
elif entity == self.end_e:
coords[0:2] = self.get_intersection(self.start_e)
coords[2:4] = self.get_intersection(entity)
self.canvas.coords(self.id, coords)
def get_intersection(self, entity):
''' 矩形の接点を求める '''
x, y = entity.get_center()
height, width = entity.rectangle.height//2, entity.rectangle.width//2
dx = self.end_e.rectangle.x - self.start_e.rectangle.x
dy = self.end_e.rectangle.y - self.start_e.rectangle.y
if entity == self.end_e:
dx, dy = -dx, -dy
if abs(dy / dx) < (height / width): # 垂直側
x_pos = x + width if dx > 0 else x - width
y_pos = y + dy * width / abs(dx)
else: # 水平側
x_pos = x + dx * height / abs(dy)
y_pos = y + height if dy > 0 else y - height
return x_pos, y_pos
class Application(tk.Tk):
''' Application class '''
def __init__(self):
super().__init__()
self.title("Connecter test 3")
self.geometry("640x320")
self.canvas = tk.Canvas(self, background="white")
self.canvas.pack(fill=tk.BOTH, expand=True)
entity1 = Entity(self.canvas, Rectangle(40, 80))
entity2 = Entity(self.canvas, Rectangle(240, 160))
Connection(self.canvas, entity1, entity2)
entity3 = Entity(self.canvas, Rectangle(420, 60))
Connection(self.canvas, entity2, entity3)
def main():
''' main function '''
application = Application()
application.mainloop()
if __name__ == "__main__":
main()
説明
まずは、2行目です。namedtupleはcollectionsモジュールに含まれていますので、まずはimportしておきましょう。これでnamedtupleを使えるようになりました。4行目でさっそく使っています。これだけで、x、y、width、heightの4つのアトリビュートをもつRectangleクラスと同等のものができあがりました。defaultsオプションは後ろの方の初期値になります。
Rectangle = namedtuple('Rectangle', ['x', 'y', 'width', 'height'],
defaults=(60, 40))
今回の場合は、widthとheightが省略された場合、これらに対し、60と40がセットされます。
これで、Entityクラスの__init__()メソッドの4つのパラメータが一つにまとまりますので、これだけで「Too many arguments」のリファクタリングは完了です。あとは、インスタンスアトリビュートも地道に変更すれば完了です。self.xをself.retangle.x、self.yをself.rectangle.y、self.widthをself.rectangle.widht、self.heightをself.rectangle.heightといった具合にすべて変更しました。あと、111行目あたりのEntity作成時の初期値部分をRectangle(40, 80)というふうに変更しました。
実行していたわかったことですが、namedtupleは名前がタプルということだけあって、不変なのですね。箱を動かしたときに、座標をセットしていたのですけど、self.rectangle.xは値を変えられませんでした。ま、名前から想像すれば、当たり前のことだったのですけどね。と、いうことで、唯一の座標を保持するための場所は、Rectangleクラスを再作成することにしました。42行目です。「Rectangle(*coords[0:2], *self.rectangle[2:4])」というふうに初期化しているのですが、namedtupleで生成したタプルは、属性が名称で参照できるだけでなく、タプルと同じように位置でアクセスすることもできるのですね。すばらしいです！
まとめ
インスタンスアトリビュートの数は7個までにおさえましょう。属性をまとめるのには、namedtupleが便利です。