2
\$\begingroup\$

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?

\$\endgroup\$
4
  • \$\begingroup\$ Not sure if you are just exclusively using this pattern for your example, but the O(n) search pattern in render() can be replaced by a O(1) search pattern with a dict. (e.g. shape_render_methods = {'circle': showCircle, 'rectangle': showRectangle}, called like this. for shape in shapes: shape_render_methods[shape.name]()) \$\endgroup\$ Aug 3 '12 at 19:48
  • \$\begingroup\$ Good observation. I considered using a dict, but I wasn't sure it was a portable solution. Do all object oriented languages guarantee that you can use a class name as the key to a dictionary? In my mind, this is a language-agnostic problem, so it should have a language-agnostic solution. \$\endgroup\$
    – Kevin
    Aug 3 '12 at 19:54
  • \$\begingroup\$ I'm not sure that they do. In any case, I think it's better that you define a name attribute for your shape class--that's what I meant to imply by shape.name. The above code was meant as an example pattern, not an actual implementation. \$\endgroup\$ Aug 3 '12 at 19:57
  • \$\begingroup\$ Oops, I must have misread your first comment. On second look, that code is indeed language agnostic. Giving each shape subclass its own name would work, but I'm wary of restating type information in a new form. Don't Repeat Yourself, as they say. \$\endgroup\$
    – Kevin
    Aug 3 '12 at 20:08
2
\$\begingroup\$

Here's one approach:

SHAPE_RENDERER = {}

def renders(shape):
    def inner(function):
        SHAPE_RENDERER[shape] = function
        return function
    return inner

@renders(Circle)
def draw_circle(circle, view):
    ...

@renders(Triangle)
def draw_triangle(triangle, view):
    ....

def render_shape(shape, view):
    SHAPE_RENDERER[shape.__class__](shape, view)
\$\endgroup\$
2
  • \$\begingroup\$ This nicely resolves the 2000-line method problem. But can this approach be taken in other OO languages that don't support decorators? In which case I imagine you'd need an initialize_SHAPE_RENDERER method which takes 2000 lines to populate the dict, and now we're back at the start. \$\endgroup\$
    – Kevin
    Aug 3 '12 at 20:12
  • \$\begingroup\$ @Kevin, this depends on what other OO language. For example, in Java you could do something with reflection or annotations. In C++ you could do some magic with macros and static objects. \$\endgroup\$ Aug 3 '12 at 21:20
1
\$\begingroup\$

Why would this not work?

#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

    def show(self, v):
        v.curve(self.x, self.y, self.radius, 0, 360)

class Rectangle(Shape):
    def __init__(self, color, x, y, width, height):
        Shape.__init__(self, color, x, y)
        self.width = width
        self.height = height

    def show(self, v):
        v.line(self.x, self.y, self.x+self.width, self.y)
        v.line(self.x+self.width, self.y, 
               self.x + self.width, self.y + self.height)
        v.line(self.x + self.width, self.y + self.height,
               self.x, self.y + self.height)
        v.line(self.x, self.y + self.height, self.x, self.y)

....

#end of model code

#view code

class TickerTapeView:
    def __init__(self, model):
        self.model = model
    def render(self):
        for shape in self.model.shapes:
            shape.show(self)

    def curve(self, x, y, radius, start, end):
        print("Curve at ({0},{1}) with radius {2} goes from {3}o to {4}o".format(x, 
              y, radius, start, end)

    def line(self, x1, y1, x2, y2):
        print("Line from ({0},{1}) to ({2},{3})".format(x1, y1, x2, y2)

#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()

GraphicalView, OscilloscopeView, and DioramaView each have to implement curve() and line(), obviously, each doing the right thing to display the shape described in its own terms.

\$\endgroup\$
1
  • \$\begingroup\$ I like the idea of passing the view into show - a nice twist on Joel's comment, that removes the need for a component that can distinguish between lines/curves/etc. I would need a View.describe(text_description) method, if I wanted TickerTapeView to print the same kind of information that it does in my original post. I'm faintly concerned that the base View class would eventually become cluttered with abstract methods like that as I add more subclasses, but for this particular example it seems entirely manageable. \$\endgroup\$
    – Kevin
    May 9 '14 at 13:58
0
\$\begingroup\$

This answer is a bit too easy, so may be a catch but I wonder: Why don't you add a .show() method to Circle class which will be equivalent of showCircle, then the same with Rectangle etc and then just:

for shape in self.model.shapes: 
    shape.show() 

or better, .show() returns sth which a render function will take care of it if you want to do fancier things:

for shape in self.model.shapes: 
    self.renderer(shape.show())
\$\endgroup\$
4
  • \$\begingroup\$ I can't put any display-specific code in my shape classes, since I have resolved to keep the implementation details of the view separate from the model. Or if you mean that show() only returns data that the view can use to distinguish between shape types, then that's just an isinstance call in disguise. \$\endgroup\$
    – Kevin
    Aug 3 '12 at 11:59
  • \$\begingroup\$ @Kevin: I wonder if show() can return non-implementation specific rendering data. Rectangle.show, for example would return something in the form of "LINE POINT1 POINT2, LINE POINT2 POINT3, LINE POINT3 POINT4, LINE POINT4 POINT1", which can be interpreted by render(). \$\endgroup\$ Aug 3 '12 at 19:43
  • \$\begingroup\$ That would indeed work very well if every shape could be precisely defined using only lines and points. However, circles and beziers can't be represented just by straight lines, so I'd need a CURVE rendering object in addition to LINE. In which case I need a renderRenderingObject method which can distinguish between concrete subclasses of the RenderingObject class, and then we're back to my original problem, albeit at a smaller scale. \$\endgroup\$
    – Kevin
    Aug 3 '12 at 20:23
  • \$\begingroup\$ Each view calls <shape>.show(). Each view also provides primitives that <shape>.show() can call to display itself in that view. The sequence of primitive calls is the same regardless of the view. So view says <circle>.show(self), which says, eg, <view>.curve(x, y, r, 0, 360), where (x,y) is the center of the circle, r is the radius, and 0, 360 indicate the portion of the curve to draw. Each view implements curve() in its own terms. The view doesn't have to know which shape it's calling and the shape doesn't have to know which view is drawing it. \$\endgroup\$
    – Tom Barron
    May 4 '14 at 1:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.