Need advice on C++ coding pattern

Question

Hi!

I have a working prototype of a game engine and right now I'm doing some refactoring. What I'm asking for is your opinion on usage of the following C++ coding patterns.

I have implemented some trivial algorithms for collision detection and they are implemented the following way:

Not shown here - class constructor is made private and using algorithms looks like Algorithm::HandleInnerCollision(...)

class Algorithm {
   // Private routines
   static bool is_inside(Point& p, Object& object) {
      // (...)
   }

   public:
      /**
       * Handle collision where the moving object should be always
       * located inside the static object
       *
       * @param  MovingObject & mobject  
       * @param  const StaticObject & sobject  
       * @return void
       * @see
       */
      static void HandleInnerCollision(MovingObject& mobject, const StaticObject& sobject) {
         // (...)
      }

So, my question is - somebody advised me to do it "the C++" way - so that all functions are wrapped in a namespace, but not in a class. Is there some good way to preserve privating if I will wrap them into a namespace as adviced?

What I want to have is a simple interface and ability to call functions as Algorithm::HandleInnerCollision(...) while not polluting the namespace with other functions such as is_inside(...)

Of, if you can advise any alternative design pattern for such kind of logics, I would really appreciate that...

Answer 1

Yes, you can use unnamed namespace:

namespace
{
   int private_variable;
}

int f()
{
   return private_variable;
}

private_variable will be seen only inside current translation unit.

But generally I do not think it is a good idea. Better use classes, OOP, components. :) The higher is better. That's the modern C++ way.

Answer 2

There are a couple of idiomatic ways of doing this. The first way is, where there are implementation files (cpp files) simply use an unnamed namespace in that file for global functions or variables. Unnamed namespaces are not really unnamed, but instead have a unique name in each translation unit which you can't type. If you're functions are inline in a header file then you can't do this - the common approach seems to be to use a namespace named detail inside your feature namespace. This is not really private, but it is common enough for devs to leave well alone.

One thing you should note - the function is_inside in your example is marked in the comments as private, but it is not actually private. By default struct members are public - in fact this is the only difference between structs and classes, so because you don't precede it with private, it is public.

Answer 3

Well, to answer your initial question, to "privatize" a function, you either use static if you want it to be local to that file, or you can put it in a private header file if you want to use it throughout your code. I.e., either have static bool is_inside(const Point& point, const Object& object); in your .cc file or put the declaration inside of algorithmsPrivate.hh.

However, really, you're writing C-like code here. If you want to do it the C++ way, you would instead add a method to the Object class:

class Object {
   ...
   bool contains(const Point& point) const;
}

One nice thing about this is it would even allow you to make the function virtual.

virtual bool contains(const Point& point) const;

Then as you implement each Object, it can have its own implementation of the contains function.

Answer 4

You can have the functions you want visible in one namespace and then have the functions like is_inside() in another namespace such as:

namespace othernamespaceHelperFunctions 
{
  ...helper functions... 
}

Just have the namespace you want to use be the only thing that uses the helper function namespace.

Answer 5

If the various functions have something to do with each other, if it's more than helping but really have a common factor such as acting on the same entity, then put them in a class. On the other hand, if a function acts on the same entity as others, but you also want to reuse it for other entities that are convertible to the first entity then extract it outside of the class:

bool same_size(const shape& a, const shape& b)
{
    return a.bounding_x == b.bounding_x && a.bounding_y == b.bounding_y;
}

The above function will work with any two objects that are convertible to or descend from shape.

Put functionality that you don't want to share with other translation units (i.e. helper functions and help objects) in an unnamed namespace. This is the C++ equivalent of the C static functions, and it instructs the compiler to keep anything inside the unnamed namespace private to this translation unit.

namespace {
bool is_inside(Point& p, Object& object)
{
    // ...
}
// ...
}

Put all the functionality that you want to share with other translation units in a named namespace. This will instruct the compiler to wrap the code with a common name, so names don't collide with external code (libraries etc.) names.

namespace kotti {
void HandleInnerCollision(MovingObject& mobject,
                          const StaticObject& sobject)
{
    // ...
}

// ...
}

Answer 6

I think you should keep it as a static class. It gives you the benefits of public/private, which you desire, and I see no downside. I've always considered it something of a hack to use namespaces the way the other answers are suggesting. In my view, namespaces are for grouping related classes and free functions, and for preventing name collisions, not for providing encapsulation.