When classes want to couple

Question

I am having an issue with 2 classes that were once nicely separated, but now they want to couple.

Without getting too much into the specifics of the problem, here it is:

I used to have a class Triangle that contained 3 space-position vertices.

class Triangle
{
    Vertex a,b,c ; // vertices a, b and c
} ;

There were many Triangle instances in the program, so each had retained their own copy of their vertices. Member functions such as getArea(), getCentroid() etc were written in class Triangle, and since each Triangle instance had copies of Vertex a, b and c, finding the area or centroid had no dependence on other classes. As it should be!

Then I wanted to move to a vertex-array/index buffer style representation, for other reasons. This means all vertices are stored in a single array located in a Scene object, and each Triangle retains only REFERENCES to the vertices in Scene, not copies of the vertices themselves. At first, I tried switching out for pointers:

class Scene
{
    std::vector<Vertex> masterVertexList ;
} ;

class Triangle
{
    Vertex *a,*b,*c ; // vertices a, b and c are pointers
    // into the Scene object's master vertex list
} ;

(In case you're wondering about the benefits, I did it for reasons mostly to do with triangles that share vertices. If *a moves then all triangles that use that vertex are updated automatically).

This would have been a really good solution! But it didn't work reliably, because std::vector invalidates pointers, and I was using a std::vector for the master vertex list in class Scene.

So I had to use integers:

class Triangle
{
    int a,b,c ; // integer index values
    // into the Scene object's master vertex list
} ;

But now I have this new coupling problem: to find its own area or centroid, class Triangle needs access to class Scene where before it did not. It seems like I've fsck`d something up, but not really.

WWYD?

Answer 1

It seems to me that your Triangle really does depend on your Scene (since its vertices are all members of that particular scene), so there is no shame in making the object do so. In fact, I would probably give the Triangle an obligatory Scene* member.

Answer 2

Assuming you have only one Scene, you can make it a singleton object and access the vertex list via static methods.

If you have multiple Scene objects, then each Triangle belongs to exactly one Scene - and it should "know" which scene it belongs to. Therefore, you should initialize each Triangle with a Scene reference, and store it as a class member.

Answer 3

Why not just have the vector in Scene just store pointers too?

std::vector<Vertex *> masterVertexList;

That way, Triangle can continue to use Vertex *'s and all you have to do is make sure that the pointers are deleted in Scene's destructor.

Answer 4

The change from uncoupled to coupled is a natural result of your decision to share vertices where possible. Previously, each triangle "owned" its vertices, and the scene (presumably) owned a bunch or triangles.

Allowing the triangles to share vertices changes that fundamental model -- when/if a vertex might be shared between two or more triangles, no one triangle can own that vertex any more. Though it's possible (e.g., with something like a shared_ptr) to have a distributed, shared ownership scheme, what you're doing right now is probably more straightforward: each vertex still has a single owner, but the owner is now the scene instead of the individual triangle.

Since a triangle is now only a convenient way of grouping some vertices in the "owning" collection instead of owning the vertices itself, it's no surprise that there's tighter coupling between the triangle and the collection that owns its vertices. If you care a lot about it, you could still hide the shared ownership to retain at least the appearance of your previous looser coupling.

The general idea would be fairly simple: instead of each triangle knowing the scene that holds the triangle's vertices, you'd create a vertex proxy class that combines a scene ID and a vertex index, so the triangle can manipulate the vertex proxy object just like it previously would have the vertex object. You don't completely eliminate the tighter coupling, but you isolate "knowledge" of the tighter coupling to a single class, that's only responsible for maintaining the appearance of looser coupling.

The obvious shortcoming of this would be that the vertex proxy objects are likely to store a fair amount of redundant data. For example, all the vertex proxies in any particular triangle are clearly representing vertices in the same scene. If you store the scene ID explicitly for each vertex proxy, you're storing three copies of the scene ID instead of one you'd have had previously. Sometimes this is worthwhile -- others it's not. If it's a real problem, you could try to come up with a way of avoiding storing the scene ID explicitly, but that's probably going to involve some tricks that aren't (even close to) language agnostic.

Answer 5

You could pass the vector to the triangle in its constructor so it can keep a reference to the vector. Then it does not need to access or know about a Scene.

typedef std::vector<Vertex> VertexContainer;

class Scene
{
    VertexContainer  masterVertexList ;  
} ;  

class Triangle  
{  
    // A references to the vertices contained in Scene.
    // A triangle no longer needs to know anything about a scene
    VertexContainer&   vertexListRef;

    // index into vertexListRef of the triangles points.
    VertexContainer::size_type  a;
    VertexContainer::size_type  b;
    VertexContainer::size_type  c;  

    public:
        Triangle(VertexContainer&           masterVertexList,
                 VertexContainer::size_type x,
                 VertexContainer::size_type y,
                 VertexContainer::size_type z)
            :vertexListRef(masterVertexList)
            ,a(x),b(y),c(z)
        {}
};

Answer 6

If you're only adding or removing to the ends of the vertex list, use a deque instead.

Answer 7

I don't think it's too bad. Triangle lost some generality and became a peripheral class of Scene, but if it's not used as an external interface (and that sort of linkage to internal buffers suggests not), that's just a natural evolution.

My solution would be similar to yours under the hood, but with more sugar.

struct Triangle
{
    Triangle( ... ) { ... }
    Vertex *a(),*b(),*c() ; // trivia: this is valid syntax! Getters adjust…
private:
    size_t ax, bx, cx; // … offsets…
    Scene *client; // … into the Scene object's master vertex list.
} ;

This way, you don't have to reorganize things in memory, and adapting old code merely requires adding () to ->a and .a, etc, which can be done by search-and-replace, and improves OO style anyway.

Or, do away with the constructor and private and make it POD.