Sign in to follow this  
AlanSmithee

Encapsulation through anonymous namespaces

Recommended Posts

Hello.

 

When programming in C++ I often (always) use some method of data encapsulation.

This used to mean defining a class, declaring variables and helper functions private and supplying a public interface of member functions:

class A
{
    public:
        int PublicFunction() { return _privateVariable; }
    
    private:
        int _privateVariable;
};

Any user of this class can create an instance of the class and use its public members.

Should implementation specific details happen to change, the public interface of the class can stay the same. This is all fine.

 

Howeverm, the more I code, the more I find myself in situations where there only needs to (or only should) ever be one instance of a class.

There are many solutions to this, such as the singleton pattern, classes with static member functions etc.

 

But somehow these designpatterns have always felt wrong to me. It feels like trying to fit the problem to the solution, where the solution always is some form of a class, be it a singleton or whatever.

 
This led me to eventually just stop using any "formal" way of data encapsulation in those situations and instead use free functions grouped in a namespace. This approach got rid of the above mentioned "problem", but introduced another problem, which is that variables and methods that should be private were now a part of the public interface*. This is obviously a far bigger issue then not liking a certain design pattern.
 
Recently, however, I stumbled across anonymous namespaces, which seems to take care of the above mentioned problem:
 
namespace // private, not accessable from outside this source file
{
    int privateVariable = 54;
 
    int PrivateHelperFunction()
    {
        // can change this to whatever i want without breaking the public interface
        return 123;
    }
}
 
namespace PublicInterface
{
    bool PerformSomeAction()
    {
        // Can access "private" varaibles and functions from here:
        return PrivateHelperFunction() == privateVariable;
    }
}
 
This seems like a great alternative to using singletons and static classes, and I am using this approach a lot now.
 
I would like to ask though, if there are any obvious drawbacks or pitfalls that I should be aware of.
One thing that makes me slightly worried is that I have been programming for quite a while in C++ and I never see this coding style being used in examples or even being talked about, which makes me think that there is something "wrong" about it.
 
potential issues that I can think of:
  • bad (or incorrect) use of namespaces
  • cannot seperate implementation over many source code files (would you even want to do this?)
  • could it make code less reusable maybe?
 
Thanks in advance for your help!
 
*if making dll's etc. I guess you could just not export those parts - but this is not the situation I am talking about here

Share this post


Link to post
Share on other sites
There's nothing wrong with this, and it's what anonymous namesakes are for. BTW, it's the same as:
static int privateVariable = 54;
 
    static int PrivateHelperFunction()
    {
        // can change this to whatever i want without breaking the public interface
        return 123;
    }
This style used to be quite common in C code, and you might even call it an ADT instead of a public interface if you came from those circles...

However, the 'there can only be one' and thus the singleton/global-state is a code smell. Why dictate that the library has to have a single global state and restrain it like that if you don't have to?

Share this post


Link to post
Share on other sites

BTW, it's the same as:

Not entirely: there's an important difference between names with static linkage and names with extern linkage. For example, templates can only be instantiated with names of extern linkage. Names with extern linkage get involved in link-time resolution, names with static linkage do not (so giving things static linkage where possible can speed up build times, sometimes remarkably).

Names at namespace level (including the :: namespace and the anonymous namespaces) by default have extern linkage. Adding the 'static' keyword gives them static linkage instead.

Still, it is the traditional C way of encapsulating things, used since the early 1970s. There are worse things to reinvent.

Share this post


Link to post
Share on other sites

Thanks guys!

 



However, the 'there can only be one' and thus the singleton/global-state is a code smell. Why dictate that the library has to have a single global state and restrain it like that if you don't have to?

 

This is something I have been thinking about a lot.
For example; in a thread discussing something related, someone talked about using multiple instances of a rendering engine for debugging purposes.
This is a very valid point and I am taking this into consideration.
 
One approach I am considering is encapsulating all state-related information into a class or struct, say "Renderer", and leave the public interface as free functions (with anonymous "helper" namespace):
 
namespace // private helper methods and variables (constants etc)
{
    int DrawHelper(const RenderAPI::Renderer& renderer)
    {
        return renderer.GetSomething();
    }
}
 
namespace RenderAPI // public renderer API
{
    class Renderer
    {
        public:
            int GetSomthing() { return _something; }
 
        private:
            int _something;
    };

    Draw(const Renderer& renderer)
    {
        DrawHelper(renderer);
    }
};
 
Which seems like a good tradeoff to me.. Think this would be overengineering?

Share this post


Link to post
Share on other sites
Global (mutable) data is generally a bad idea, even if inside an anonymous namespace. This is because your functions then are depending on something that the user can't see, and which may change their behavior in unexpected ways - not to mention that the whole scheme breaks down entirely when threads are involved.

Singletons are also generally frowned upon for the same reasons - there is usually no benefit to having a singleton vs. making an instance of a class and passing it around. And singleton classes can make it easy to create invisible dependencies. They also make it incredibly hard to "mock" for doing testing.

I'm not sure what you're getting at with your example with the renderer though. Classes are designed to be a mechanism which allows you to encapsulate data and modify it only though an approved interface. Your "DrawHelper" function in this case has no purpose.

It is also unclear where you are sticking these anonymous namespaces. They serve little to no purpose in headers where anyone who includes the header gets instant access to them - not exactly "private".

Anyway.

To try to get a little bit on topic - encapsulation is what classes are for. So use classes. Anonymous namespaces are not for encapsulation, they are for avoiding name collisions in translation units (cpp files) that are linked together. Free functions are used whenever you do not need access to a class' private members. Singletons and mutable global state have little to no use in modern C++ - you can almost always make cleaner, more maintainable, and more testable code without them. Edited by SmkViper

Share this post


Link to post
Share on other sites

Hi, thanks for your input.

 

Yes, ofcourse the anonymous namespace would be in the source file, and the example was only to show what I mean.

I do understand your reasoning though and I agree that if I am going to use classes or structs, the need for anonymous namespaces more or less goes away.

Share this post


Link to post
Share on other sites

Global (mutable) data is generally a bad idea, even if inside an anonymous namespace. This is because your functions then are depending on something that the user can't see, and which may change their behavior in unexpected ways - not to mention that the whole scheme breaks down entirely when threads are involved.

 

True, but the functions themselves are stateless. Free functions is not the same thing as global state. Sometimes it makes a lot of sense to have free functions that are associated with a class but do not belong to it. For instance, you could use free functions to promote encapsulation by doing what AlanSmithee suggested where free functions would be the public interface and the actual class would be a hidden implementation detail, similar to how C's FILE structure is used. 

 

Anonymous namespaces don't really contribute to that, though. As you say, they're more for fixing naming collisions.

Share this post


Link to post
Share on other sites

Global (mutable) data is generally a bad idea, even if inside an anonymous namespace. This is because your functions then are depending on something that the user can't see, and which may change their behavior in unexpected ways - not to mention that the whole scheme breaks down entirely when threads are involved.

 
True, but the functions themselves are stateless. Free functions is not the same thing as global state. Sometimes it makes a lot of sense to have free functions that are associated with a class but do not belong to it. For instance, you could use free functions to promote encapsulation by doing what AlanSmithee suggested where free functions would be the public interface and the actual class would be a hidden implementation detail, similar to how C's FILE structure is used. 
 
Anonymous namespaces don't really contribute to that, though. As you say, they're more for fixing naming collisions.


You're basically saying the same thing smile.png I was trying to point out that if you have mutable global state, then a function could use it, thereby changing its behavior based on the changing state that is not passed into the function itself, or necessarily visible to the user.

If a function only operates on public-facing members of a class, then it should not be a part of the class smile.png

If a function modifies or uses modifiable global variables, then it's really a class member of a class you can't see, don't know about, and can't predict.

Share this post


Link to post
Share on other sites

Hi!

 

Thanks to everyone for contributing, I have learnt a few things.

 

Ravyne, you are pretty much spot on with everything you say, except me being against the idea of having free functions as a part of the interface - this is actually what I want to achive - sorry if I was unclear. What I did have a problem with was hiding away things that should not be in the public interface. This was a problem to me, because I was not using any form of  formal encapsulation (like a class). I was essentially moving from one extreme (using classes for everything) to another extreme (don't use classes at all). I think that was what was making it "not feel right".

 

So, gathering what has been said in this thread, this is the current solution I am considering: (psuedo code example)

// Public interface of texture manager
namespace TextureManager
{
    class Instance
    {
        public:
            void ChangeTheStateOfThisInstance()
            {
                 ++_secretVariable;
             }
 

            void ChangeTheStateOfThisInstanceTwo()
            {
                 --_secretVariable;
             }
 

            void ChangeTheStateOfThisInstanceThree()
            {
                 _secretVariable *= _secretVariable;
             }
 
         private:
             int _secretVariable = 0;
    };
 
    void IAmAPartOfTheTextureManagerInterface(Instance textureManager)
    {
        textureManager.ChangeTheStateOfThisInstance();
        textureManager.ChangeTheStateOfThisInstanceTwo();
        textureManager.ChangeTheStateOfThisInstanceThree();
    }
};
 
// Usage
int main()
{
    TextureManager::Instance textureManager;
 
    TextureManager::IAmAPartOfTheTextureManagerInterface(textureManager);
 
    // and/or
 
    textureManager.ChangeTheStateOfThisInstanceTwo();
   
    ...
 
    return 0;
}

This feels good to me, and if I am consistent with how I provide the public interface of different functionality, I think it could work well.

I do however realize that not all will agree with the usage of namespaces or the naming conventions used in the above example.

No need for any anonymous namespace here atleast. Should the need arise, it would be another tool in the toolbox though.

 

Naming conventions aside, I think this is pretty much what Ravyne is talking about?

Any feedback on this approach?

 

Thanks again!

Share this post


Link to post
Share on other sites

 

Naming conventions aside, I think this is pretty much what Ravyne is talking about?

Any feedback on this approach?

 

That's pretty good, though (again to push the point I was getting at earlier) you could make it even more encapsulated, at the expense of some extra code:

 

[source]

// Public interface of texture manager (texture.h)
namespace TextureManager
{
    class Instance;
    void IAmAPartOfTheTextureManagerInterface(Instance& textureManager);
    void DoOtherThingWithTextureManager(Instance& textureManager);
}
 
// Implementation of public interface and definition of private interface (Texture.cpp)
namespace TextureManager
{
    class Instance
    {
        public:
            void ChangeTheStateOfThisInstance()
            {
                 ++_secretVariable;
             }
 
 
            void ChangeTheStateOfThisInstanceTwo()
            {
                 --_secretVariable;
             }
 
 
            void ChangeTheStateOfThisInstanceThree()
            {
                 _secretVariable *= _secretVariable;
             }
 
         private:
             int _secretVariable = 0;
    };
 
    void IAmAPartOfTheTextureManagerInterface(Instance textureManager)
    {
        textureManager.ChangeTheStateOfThisInstance();
        textureManager.ChangeTheStateOfThisInstanceTwo();
        textureManager.ChangeTheStateOfThisInstanceThree();
    }
 
    void DoOtherThingWithTextureManager(Instance& textureManager)
    {
        textureManager.ChangeTheStateOfThisInstanceTwo();
    }
}
 
// Usage (main.cpp)
int main()
{
    TextureManager::Instance textureManager;
 
    TextureManager::IAmAPartOfTheTextureManagerInterface(textureManager);
 
    // and/or
    // no longer doing this
    //textureManager.ChangeTheStateOfThisInstanceTwo();
    // instead do this
    DoOtherThingWithTextureManager(textureManager);
    ...
 
    return 0;
}
[/source]

 

Now note that only texture.cpp knows the definition of the TextureManager. All anyone else knows about is the functions in that namespace you've provided.

Share this post


Link to post
Share on other sites

Thanks for your additional replies!

 

I didn't know that your could forward declear a class like that in the header and leave both declaration and definition of it's body to the source file.. thought that would cause the compiler to throw a multiple declarations error.. cool!

 

I really like the example in your last post, Oberon_Command. It achives exactly what I wanted to with the functions in the anonymous namespace, but this way I am enable to have multiple instances of a type with a unified API through free functions.

 

Thanks for your help.

 

(and yes, I would pass it by ref had I not written the code in the browser and forgotten about it :))

Share this post


Link to post
Share on other sites

I didn't know that your could forward declear a class like that in the header and leave both declaration and definition of it's body to the source file.. thought that would cause the compiler to throw a multiple declarations error.. cool!

 

That's actually necessary in some cases. Consider the following examples of circular references:

 

[source]

struct Foo{

    Bar* x; // error, Bar is not declared

};

 

struct Wololo;

struct Bar {

    Wololo* x; // no error, Wololo was forward declared

};

 

struct Wololo {

    Bar* x; // no error, we already declared Bar above

};

[/source]

Edited by Oberon_Command

Share this post


Link to post
Share on other sites

 


I didn't know that your could forward declear a class like that in the header and leave both declaration and definition of it's body to the source file.. thought that would cause the compiler to throw a multiple declarations error.. cool!

 

That's actually necessary in some cases. Consider the following examples of circular references:

 

[source]

struct Foo{

    Bar* x; // error, Bar is not declared

};

 

struct Wololo;

struct Bar {

    Wololo* x; // no error, Wololo was forward declared

};

 

struct Wololo {

    Bar* x; // no error, we already declared Bar above

};

[/source]

 

 

Yeah, I know about foward declaration, but I thought you had to re-decleare the struct or class in another header to not get "multiple declarations" exception when more than one file uses the class/struct.

Share this post


Link to post
Share on other sites

The hard part is managing instantiation. To instantiate the object, C++ needs to know it's size. This means that in Oberon_Command's example, either the main.cpp file needs to be able to include a special header that gives the class definition, or you need to switch to dynamic allocation,e.g. the original header contains a factory function to return something like std::unique_ptr<Instance>.

 

That interface is very C like (opaque struct). Consider also the PIMPL idiom.

Share this post


Link to post
Share on other sites

 

 


I didn't know that your could forward declear a class like that in the header and leave both declaration and definition of it's body to the source file.. thought that would cause the compiler to throw a multiple declarations error.. cool!

 

That's actually necessary in some cases. Consider the following examples of circular references:

 

[source]

struct Foo{

    Bar* x; // error, Bar is not declared

};

 

struct Wololo;

struct Bar {

    Wololo* x; // no error, Wololo was forward declared

};

 

struct Wololo {

    Bar* x; // no error, we already declared Bar above

};

[/source]

 

 

Yeah, I know about foward declaration, but I thought you had to re-decleare the struct or class in another header to not get "multiple declarations" exception when more than one file uses the class/struct.

 

 

Don't forget that "headers" are not really a thing from the perspective of the compiler. When you #include something, the compiler is essentially copy-pasting that file into the current translation unit. Even if you put the class definition in a single header that gets included in multiple files, the compiler sees you as defining the same class multiple times anyway. The "multiple declarations" error only comes into play if the class definition occurs twice in the same translation unit or if you try to define a global twice in two different files (because the compiler needs to set aside some memory for the global and needs to know which translation unit the global came from, unlike class definitions which take up no memory).

Edited by Oberon_Command

Share this post


Link to post
Share on other sites

Thanks again for your explanations!

 

I feel that I have learnt some great stuff from this thread!

 

I do have a follow up question however, that is a bit more implementation specific:

 

How come this works:

 

Test.h

namespace Test
{ 
    class Instance;
 
    Instance* CreateInstance();
}

 

Test.cpp

#include "Test.h"
 
namespace Test
{
    class Instance
    {
        public:
            Instance() {};
            ~Instance() {};
 
        private:
            int one = 1;
    };
 
    Instance* CreateInstance()
    {
        return new Instance();
    }
}

 

main.cpp

#include <iostream>
#include "Test.h"
 
int main()
{
    auto test = Test::CreateInstance();
 
    if (test != nullptr)
    {
        std::cout << "deleting" << std::endl;
 
        delete test;
    }
 
    return 0;
}

 

But this doesn't:

 

Test.h

#include <memory>
 
namespace Test
{ 
    class Instance;
 
    std::unique_ptr<Instance> CreateInstance();
}

 

Test.cpp

#include "Test.h"
 
namespace Test
{
    class Instance
    {
        public:
            Instance() {};
            ~Instance() {};
 
        private:
            int one = 1;
    };
 
    std::unique_ptr<Instance> CreateInstance()
    {
        return std::make_unique<Instance>();
    }
}

 

main.cpp

#include "Test.h"
 
int main()
{
    auto test = Test::CreateInstance();
 
    return 0;
}

 

With error "use of undefined type Test::Instance"?

 

From what I can gather by reading around is that you should be able to create a unique_ptr<T> as long as there is a constructor and destructor defined for T?

I am using the compiler that comes with VS for testing this, could it be compiler specific?

 

Thanks again! =)

Share this post


Link to post
Share on other sites
Consider what happens when your second test.h is included in another unit. The compiler has to generate the code for destroying T from the smart ptr, but it only has the forward declaration of T available.

The compiler cannot guarantee to do the right thing with the destructor then because it does not know, for example, if T derives from another class.

So it's not possible to use a smart ptr that calls destructor of T if T is only forward declared.

Share this post


Link to post
Share on other sites
Neither example actually works. Trying to build the first example on the compiler I have in front of me right now (MSVC2013) produces a warning:
 
warning C4150: deletion of pointer to incomplete type 'Test::Instance'; no destructor called
In other words - for some reason the C++ standard (or at least this implementation of it) allows deletion of incomplete types, but will not call the destructor. This is pretty disastrous if your destructor actually did any work.

Example 2 fails with the expected error of trying to use an incomplete type, which is not allowed in the template instantiation. Note that even though it fails, it also gives the same warning as above, except it points at the default_delete::operator() implementation instead of into main.

(The destructor being inline should never matter) Edited by SmkViper

Share this post


Link to post
Share on other sites


From what I can gather by reading around is that you should be able to create a unique_ptr as long as there is a constructor and destructor defined for T?
I am using the compiler that comes with VS for testing this, could it be compiler specific?

 

In the code which only includes the header, the constructor and destructor are not defined. The problem with destruction is the same as the one with construction - you need to make a DeleteInstance() function. There aren't many cases I can think of for using a C-style opaque struct in C++, but at least when it comes up in C, creating construction and destruction functions is very common.

 

If you decide to use this pattern, your unique_ptr could have a custom deleter class as its second template parameter, which calls DeleteInstance().

Share this post


Link to post
Share on other sites
Generally, if you've obtained an object via something other than new, you should release it with something other than delete.

This means that as well as your Create function, you should have a matching Destroy/etc function.

If you want to use smart pointers, you can probably then configure them to call your custom release function, rather than the global delete operator.

Share this post


Link to post
Share on other sites

Hello again.

 

Taking into consideration all that has been said in this thread I have been improving my design through several iterations, trying out different paradigms like the pimpl idiom etc.

 

I really liked the encapsulation that the pimpl idiom provided, but I had problems with how the code was structured and I did not find it very intuitive.

 

The style I have decided to use is a kind of opaque pointer that works just like the pimpl idiom, except that instead of having a "hidden" implementation class, there is instead a "hidden" accessor class that is a friend of the opaque pointer. This makes the code much easier to read (for me) and the usage fits very well with the SDL interface, which is what I am using for windowing, events etc.

 

I also moved away from using namespaces to group functionality, as function signatures are (or should be) clear enough to show how they should be used

 

This is how I implemented it: (psuedocode)

 

Test.h

class TextureManager
{
    friend class TextureManagerAccessor;
 
    TextureManager()
    {
        _textures.reserve(1000);
    }
 
    ~TextureManager()
    {
        for (auto& kvp : _textures)
        {
            if (kvp.second == nullptr)
            {
                continue;
            }
 
            delete kvp.second;
            kvp.second = nullptr;
        }
    }
 
    std::unordered_map<unsigned int, SDL_Texture*> _textures;
};
 
TextureManager* CreateTextureManager();
DestroyTextureManager(TextureManager*);
DoSomethingFunny(TextureManager*);

Test.cpp

class TextureManagerAccessor
{
    public:
        static TextureManager* Create();
        static void Destroy(TextureManager*);
};
 
TextureManager* TextureManagerAccessor::Create()
{
    return new TextureManager();
}
 
void TextureManagerAccessor::Destroy(TextureManager* textureManager)
{
    delete textureManager;
    textureManager = nullptr;
}
 
TextureManager* CreateTextureManager()
{
    return TextureManagerAccessor::Create();
}
 
DestroyTextureManager(TextureManager* textureManager)
{
    if (textureManager == nullptr)
    {
        return;
    }
 
    TextureManagerAccessor::Destroy(textureManager);
}
 
DoSomethingFunny(TextureManager* textureManager)
{
    // something funny
}

Main.cpp (usage)

int main()
{
    auto textureManager = CreateTextureManager();
    
    DoSomethingFunny(textureManager);
 
    DestroyTextureManager(textureManager);
 
    return 0;
}

I am very happy with how this turned out.

I realize some people are against using a friend class in this manner, but I do not see any problem with it, as I consider the instance, the accessor and the related free functions as one "unit".

 

Thanks for all your help. =)

Edited by AlanSmithee

Share this post


Link to post
Share on other sites

Hi.

 

Sorry for doubble posting, but since it was a few days since my last post, I thought it would be more clear with an additional post for anyone reading this in the future.

 

So.. After using the style explained in my previous post for a while it turned out i still wasn't completely happy with it.

The thing that bugged me the most with it was that many of the free functions were just forwarding to the respective static method in the Accessor class.

 

After looking around some more, I came across this article http://www.drdobbs.com/cpp/how-non-member-functions-improve-encapsu/184401197 which really hit the spot for me.

Funny thing is that I own both Effective C++ and Effective STL and have read them both as well as that article before, but it was first now, that I have actually had a need to think about this, that this advice made sense to me.

 

I will not be posting any code examples this time, but I have decided to simply follow Scott Meyers advice and supply a public interface of free functions when possible and friend functions when I have to.

Edited by AlanSmithee

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this