1
\$\begingroup\$

I'm designing a class that holds values representing a 3D scene. This includes lights, cameras, meshes, materials, etc.

The way how I'm setting it up is that each "thing" has a name. A camera has a name, a light has a name, etc. But they also have an "interface" to access the object via pointer instead of having to name it each time something needs to be changed.

For example:

Light *light = scene.getLight("light2");
light->setBrightness(2.0f);

The problems is what Light is. Ideally, it's "proper C++" where it's a pointer directly to the internal object. However, these classes are closely tied to how the scene class works, which makes maintainability a mess. It's seems much easier to just have private implementations that do all the dirty work and only let the user receive pointers to an interface.

What I want to do is make Light an interface (struct with virtual functions, no data, no non-virtual functions except empty constructor) and implement the internal details as a private class inside the scene class. While this makes it more maintainable, it comes at the cost of using polymorphism where I'm not dealing with more than one implementation.

Any thoughts or ideas are helpful.

\$\endgroup\$
4
  • \$\begingroup\$ Will this work: scene.SendMessageToMediator(NVPair("SetBrightness", "light2", "2.0f")); ? \$\endgroup\$
    – mythos
    Commented Nov 27, 2015 at 15:23
  • \$\begingroup\$ @sakul_ca That's probably worse because 1. I can't call a function by the name unless I map a function pointer to it, but that in itself is a performance issue and 2. it's quite messy. \$\endgroup\$
    – user112513312
    Commented Nov 27, 2015 at 16:57
  • \$\begingroup\$ It is a design pattern that controls how objects interact. I feel that it will work best in your situation. You already know about "scene" so why not implement it with an abstract mediator? You already have the naming convention in use. Also instead of implementing private details of the lights. You may want to use an Abstract Factory to create the lights. That way the scene doesn't have to know about each of the light types. It just knows the Light Interface type, and calls what it needs to know. ILight* newLight = lightfactory->CreateLightByType("PointLight"); \$\endgroup\$
    – mythos
    Commented Nov 28, 2015 at 4:31
  • \$\begingroup\$ By the way, please consider returning references rather than using old C pointers. You could also template the get method. Light& light = scene.get<Light>("light2"); \$\endgroup\$
    – danijar
    Commented Nov 29, 2015 at 1:03

1 Answer 1

Reset to default
2
\$\begingroup\$

Unless you really have a good reason for having a pure virtual interface, I wouldn't bother. Nothing to do with performance, just the fact that with the interface setup, you have one extra class declaration that needs maintaining.

If your scene objects are already encapsulated inside classes, there's no gain in having a virtual interface just to hide a couple private methods and data (which are not accessible outside the class anyways). And it doesn't make it more maintainable, makes it less, because you have two identical public interfaces that have to be kept in sync.

Now if you do need to have multiple implementations, then it is a viable approach. Assuming you'd have a Light as interface and then DirectionalLight, PointLight, etc as implementations, it makes sense, but while it is a single concrete impl, just use it directly.

As a side note, to make it "proper C++", consider using smart pointers or even plain references instead of raw pointers to objects.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged .