1

I'm building my own Canvas-style JPanel subclass which will draw a graph of nodes and arcs. As part of this application I am delegating the drawing of the nodes to a sprite class Node, i.e

Class Visualiser extends JPanel {

    ...

    paintComponent(Graphics g) {
        ...
        node.draw(g);
        ...
    }
}

But I also have a class Node for a data structure. I'm not concerned about the nomenclature, I can call one NodeSprite to avoid conflicts, etc...

What I am wondering is whether to merge the data structure and the sprite class into one, as logically they both describe the same real-world thing, or if doing this would have any negative side effects such as performance, or general bad design.

Any suggestions?

Adam
  • 5,091
  • 5
  • 32
  • 49
  • Why do your `Node` sprites know how to draw themselves, or am I misunderstanding your design? – Tyler Holien Feb 06 '11 at 14:33
  • The concept in my mind is to pass off the logic of drawing a node into its own class. That way if I wanted a different type of sprite, I wouldn't have to put more and more logic into my JPanel subclass. Do you have a better design pattern in mind? – Adam Feb 06 '11 at 14:41

3 Answers3

3

If a NodeSprite has behavior other than knowing how to print itself, it would violate the single responsibility principle. If all it knows how to do is draw, I would keep it that way and consider renaming it NodeSpritePrinter or something similar.

Tyler Holien
  • 4,791
  • 2
  • 19
  • 14
  • 1
    @Tyler I see your point, it just seems like this would add a lot of communication between the classes that could be avoided. Surely part of the reason interfaces exist in Java is to allow classes to have multiple responsibilities? – Adam Feb 06 '11 at 15:59
  • @Adam - A huge component to the concept of OO is that objects can communicate with each other. Communication between objects is a _good_ thing. – Tyler Holien Feb 06 '11 at 17:16
  • @Tyler Yes, I suppose. I am thinking this might also have performance overhead due to keeping a NodeSprite for each Node, and keeping this in sync with the whole list of Nodes as this changes. – Adam Feb 06 '11 at 18:11
  • 1
    @Adam - Does your `Node` data structure have any behavior? Does your `NodeSprite` have any behavior other than drawing? – Tyler Holien Feb 06 '11 at 19:03
  • @Adam - And I was thinking something more like `nodeSpritePrinter.draw(node)`. That way you only have one `NodeClassPrinter` instance and if you have subclasses of `Node` you can design them in such a way that the `NodeSpritePrinter` (or whatever) can still draw them. – Tyler Holien Feb 06 '11 at 19:11
  • @Tyler The Node class has behaviour in that it has methods to link itself to other nodes, and to change its name. It also can draw itself as right now, that's the only class, but I'm considering changing it. – Adam Feb 06 '11 at 23:34
  • 1
    @Tyler I'm not entirely sure I understand the wording of Printer in Node class printer, however wouldn't it be possible to have a method draw(), which (in any subclasses) may or may not be overridden - this would still allow subclasses to be drawn by one single piece of code? – Adam Feb 06 '11 at 23:37
  • @Adam - I only keep using "printer" because it sounds better than `NodeDrawingThing`. – Tyler Holien Feb 07 '11 at 03:27
1

If a NodeSpite just draws a Node. Then I would be very tempted to combine them. Sometimes it makes good design sense to separate the view from the model, but there are definitely cases where doing that creates unnecessary complexity.

Also, Node sounds like a very generic name. It might be good to give it a more specific name anyway.

jzd
  • 23,473
  • 9
  • 54
  • 76
  • Thanks. You say if it just draws a Node - what else might it do, typically? – Adam Feb 06 '11 at 14:40
  • Also, I'm not sure if there is a more specific name. There is no particular domain for which I'm drawing a graph. All I can think of is Node and Vertex.. – Adam Feb 06 '11 at 15:55
  • @Adam, I wasn't sure if you had other items that NodeSpite drew/represented. Node might be a fine name now that I look over this again. – jzd Feb 07 '11 at 12:31
0

The downside would be mixing the painting code with the domain logic of the Node. That wouldn't be a disaster, but it would make the code less clear to read. The pattern of having parallel sets of classes for modelling the domain objects and their presentation is an old and honourable one, and indeed is used in the AWT itself - for every component, there's a corresponding 'native peer'. I'd take the two-classes approach; it separates responsibilities better, and has no real downside.

Tom Anderson
  • 46,189
  • 17
  • 92
  • 133