Исходный код
if isinstance(labels, str):
try:
labels = self.vs.get_attribute_values(labels)
except KeyError:
labels = [x+1for x in xrange(self.vcount())]
elif labels isNone:
labels = [""] * self.vcount()
if isinstance(colors, str):
try:
colors = self.vs.get_attribute_values(colors)
except KeyError:
colors = ["red"for x in xrange(self.vcount())]
if isinstance(shapes, str):
try:
shapes = self.vs.get_attribute_values(shapes)
except KeyError:
shapes = [1]*self.vcount()
if isinstance(edge_colors, str):
try:
edge_colors = self.es.get_attribute_values(edge_colors)
except KeyError:
edge_colors = ["black"for x in xrange(self.ecount())]
if not isinstance(font_size, str):
font_size = "%spx" % str(font_size)
else:
if ";" in font_size:
raise ValueError, "font size can't contain a semicolon"
vc = self.vcount()
while len(labels)<vc: labels.append(len(labels)+1)
while len(colors)<vc: colors.append("red")
#.........
for eidx, edge in enumerate(self.es):
print >>f, " <g transform=\"translate(%.4f,%.4f)\" fill=\"%s\" stroke=\"%s\">" % (x2, y2, edge_colors[eidx], edge_colors[eidx]
Что не так в исходном коде
Неоднозначное использование переменных
Переменные labels
, colors
, shapes
, edge_colors
используются в двух смыслах: как массив значений некоего свойства, либо как строковый ключ в хранилище, из которого этот массив извлекается.
Такая двусмысленность не годится: она затрудняет понимание алгоритма, резко повышает вероятность багов и затрудняет их устранение.
Для каждого свойства нужно завести пару переменных: отдельную для массива значений свойств и отдельную — для ключа, по которому значения свойств извлекаются из хранилища.
Смешанные знания
В одной простыне кода смешаны знания о том, как формировать массивы значений свойств для меток, цвета, формы и цвета границы, размера шрифта, а затем и формирования фигур на основе сформированных массивов.
Более здоровый подход — разместить код для формирования каждого набора данных, которые здесь выглядят совершенно самостоятельными, в самостоятельные же методы, чтобы каждый метод знал только одну вещь: как сформировать значения цвета фигур, как сформировать метки фигур, как сформировать фигуры и т. д.
Невнятные названия полей
Названия self.vs
, self.vcount
, self.es
, self.ecount
совершенно ничего не сообщают о своём предназначении, для его выяснения придётся разгрести много другого кода.
Сомнительная архитектура
Думаю, не сильно ошибусь, если предположу, что элементы массивов labels
, colors
, shapes
соответствуют неким геометрическим объектам, а именно: labels[0]
, colors[0]
, shapes[0]
— это свойства первого объекта, labels[0]
, colors[0]
, shapes[0]
— второго и т. д. Если так, то очень правильно будет завести отдельный клас «геометрический объект» со свойствами «метка», «форма» и «цвет».
То же самое с edge_colors
, который относится к группам объектов, судя по тегу <g>
, с помощью которого они рендерятся.
Выявление в своей предметной области сущностей и создание под них самостоятельных классов позволяет проделывать массу удивительных вещей, в частности, правильно распределять по коду знания о том, как реализуются различные моменты в проекте.
Теория
- Принцип единственного знания
- Объектно-ориентированный анализ