I have a program that displays colorful shapes to the user. It is designed so that it is easy to add new shapes, and add new kinds of views.
Currently, I have only two shapes, and a single text-based view. In the near future, I'm going to implement a Triangle
shape and a BezierCurve
shape.
I'm also going to implement these views:
GraphicalView
- uses a graphics library to render shapes on screen.OscilloscopeView
- draws the shapes on an oscilloscope.DioramaView
- a sophisticated AI directs robotic arms to construct the scene using construction paper, string, and a shoebox.
The MVC pattern is essential here, because otherwise I'd have a big intermixed tangle of oscilloscope and AI and Graphics library code. I want to keep these things as separate as possible.
#model code
class Shape:
def __init__(self, color, x, y):
self.color = color
self.x = x
self.y = y
class Circle(Shape):
def __init__(self, color, x, y, radius):
Shape.__init__(self, color, x, y)
self.radius = radius
class Rectangle(Shape):
def __init__(self, color, x, y, width, height):
Shape.__init__(self, color, x, y)
self.width = width
self.height = height
class Model:
def __init__(self):
self.shapes = []
def addShape(self, shape):
self.shapes.append(shape)
#end of model code
#view code
class TickerTapeView:
def __init__(self, model):
self.model = model
def render(self):
for shape in self.model.shapes:
if isinstance(shape, Circle):
self.showCircle(shape)
if isinstance(shape, Rectangle):
self.showRectangle(shape)
def showCircle(self, circle):
print "There is a {0} circle with radius {1} at ({2}, {3})".format(circle.color, circle.radius, circle.x, circle.y)
def showRectangle(self, rectangle):
print "There is a {0} rectangle with width {1} and height {2} at ({3}, {4})".format(rectangle.color, rectangle.width, rectangle.height, rectangle.x, rectangle.y)
#end of view code
#set up
model = Model()
view = TickerTapeView(model)
model.addShape(Circle ("red", 4, 8, 15))
model.addShape(Circle ("orange", 16, 23, 42))
model.addShape(Circle ("yellow", 1, 1, 2))
model.addShape(Rectangle("blue", 3, 5, 8, 13))
model.addShape(Rectangle("indigo", 21, 34, 55, 89))
model.addShape(Rectangle("violet", 144, 233, 377, 610))
view.render()
I'm very concerned about the render
method of TickerTapeView
. In my experience, whenever you see code with a bunch of isinstance
calls in a big if-elseif
block, it signals that the author should have used polymorphism. But in this case, defining a Shape.renderToTickerTape
method is forbidden, since I have resolved to keep the implementation details of the view separate from the model.
render
is also smelly because it will grow without limit as I add new shapes. If I have 1000 shapes, it will be 2000 lines long.
Is it appropriate to use isinstance
in this way? Is there a better solution that doesn't violate model-view separation and doesn't require 2000-line if
blocks?
render()
can be replaced by a O(1) search pattern with adict
. (e.g.shape_render_methods = {'circle': showCircle, 'rectangle': showRectangle}
, called like this.for shape in shapes: shape_render_methods[shape.name]()
) \$\endgroup\$name
attribute for your shape class--that's what I meant to imply byshape.name
. The above code was meant as an example pattern, not an actual implementation. \$\endgroup\$name
would work, but I'm wary of restating type information in a new form. Don't Repeat Yourself, as they say. \$\endgroup\$