4
\$\begingroup\$

I have created template class to easily get the current number of instances a class has. It's threadsafe and I tried to make it as hard as possible to abuse.

#include <atomic>
#include <cstdint>
#include <type_traits>


template<typename t_derived>
struct countable
{
    friend t_derived;

    struct counter
    {
        friend countable<t_derived>;

    private:
        static std::atomic<uint32_t> s_counter;
    };

    static uint32_t instances()
    {
        static_assert(std::is_base_of<countable<t_derived>, t_derived>::value,
                      "Trying to get count of non counted class");
        return counter::s_counter;
    }

private:
    countable()
    {
        ++counter::s_counter;
    }

    countable(const countable&)
    {
        ++counter::s_counter;
    }

    ~countable()
    {
        --counter::s_counter;
    }
};

template<typename t_derived>
std::atomic<uint32_t> countable<t_derived>::counter::s_counter(0);

Edit:

There seem to be some misunderstandings about this class. I'm sorry, I should've explained it a bit. Here is an example main.cpp. It requires at least C++11:

#include <iostream>

// paste implementation here


struct counted1 : countable<counted1>
{
    counted1()
    {
        std::cout << "counted1::counted1()\n";
    }
    ~counted1()
    {
        std::cout << "counted1::~counted1()\n";
    }
};

struct counted2 : countable<counted2>
{
    counted2()
    {
        std::cout << "counted2::counted2()\n";
    }
    ~counted2()
    {
        std::cout << "counted2::~counted2()\n";
    }
};


int main()
{
    std::cout << "counted1 [" << counted1::instances() << "]\n";
    std::cout << "counted2 [" << counted2::instances() << "]\n";

    counted1 s1;

    std::cout << "counted1 [" << counted1::instances() << "]\n";
    std::cout << "counted2 [" << counted2::instances() << "]\n";

    {
        counted1 s2;
        counted2 s3;

        std::cout << "counted1 [" << counted1::instances() << "]\n";
        std::cout << "counted2 [" << counted2::instances() << "]\n";
    }
    std::cout << "counted1 [" << counted1::instances() << "]\n";
    std::cout << "counted2 [" << counted2::instances() << "]\n";

    return 0;
}

Why did I make the ctors private & not protected? Simply if they were protected every class could inherit from it. Making them private & declaring the template parameter a friend enforces the correct use of the crtp pattern:

struct my_counted_struct : countable<some_other_struct>{};   // error!

This has the side effect that all actual private data (s_counter) is also exposed to the derived class, so this would be possible:

struct my_struct : countable<my_struct>
{
    void do_something_nasty()
    {
        countable<my_struct>::s_counter = 0;
    }
};

That's why the actual private data is in the inner class counter which declares countable a friend, giving it access to its private data.

\$\endgroup\$
  • \$\begingroup\$ Looks interesting. Please include a main so we can compile it and see it working. Thanks! \$\endgroup\$ – Cris Luengo May 12 '18 at 18:16
  • \$\begingroup\$ Yeah, should've done that. I'll add an example. \$\endgroup\$ – HenrikS May 13 '18 at 10:11
2
\$\begingroup\$

Nice use of the "Curiously Recurring Template Pattern".

The trouble with your design, is that any user of your class needs to declare their own version of the s_counter. Not strictly a problem but it does make it harder to use.

I would use static function variable as the counter. That way you don't actually need to declare your own version.

    struct counter
    {
        private:
            // Should not be used by friends.
            static std::atomic<uint32_t>& getCounterInstance() {
                static std::atomic<uint32_t>  counter = 0;
                return counter;
            }
    };

As counter is a private member of countable. Nobody else can see it (let alone access it). So you don't need to make countable a friend. Just use some standard public functions to get you accesses.

template<typename t_derived>
class countable
{
    struct counter
    {
        private:
            // Should not be used by friends.
            static std::atomic<uint32_t>& getCounterInstance() {
                static std::atomic<uint32_t>  counter = 0;
                return counter;
            }

       public:
            // Handy access methods:
            static void incrementCounter() {
                ++getCounterInstance();
            }
            static void decrementCounter() {
                --getCounterInstance();
            }
    };
};

The only person that has access to incrementCounter() and decrementCounter() are members of countable.

You got the default constructor and the copy constructor.

    countable()
    {
        ++counter::s_counter;
    }

    countable(const countable&)
    {
        ++counter::s_counter;
    }

But you forgot the move constructor.

    countable(countable&&)
    {
        counter::incrementCounter();
    }
\$\endgroup\$
  • \$\begingroup\$ There is no need for users of this class to declare their own versions of s_counter. Use of local static variable will only lead to some extra overhead. \$\endgroup\$ – VTT May 12 '18 at 21:40
  • \$\begingroup\$ @VTT If you use a static member variable it needs to be instantiated. So you must manually declare it somewhere. What overhead is there for a local static variable? \$\endgroup\$ – Martin York May 13 '18 at 15:39
  • \$\begingroup\$ In case of OP's code s_counter is properly instantiated, so there is absolutely no need for uses of his class to declare their own s_counter. Local static variables suffer from two kinds of overhead: (1) for each local static variable a global Boolean variable is implicitly created to flag whether that local variable has been initialized; (2) access to local static variable involves check of that Boolean flag upon scope entrance. Though hopefully modern compilers should be able to optimize away all that stuff in case of 0-initialized atomic int avoiding local static is still a good idea. \$\endgroup\$ – VTT May 13 '18 at 19:24
  • \$\begingroup\$ @VTT Your description of how local static variables work is an assumption on implementation. Sure that is one way to do it but it is not required by the standard and there are several other ways. Also this is insignificant compared to other issues (especially when you think about branch prediction). So basically your assertions are naive at best. \$\endgroup\$ – Martin York May 13 '18 at 19:31
  • \$\begingroup\$ @VTT sure the OP code works as long as you only include the header file in one place. Which in real life is not going to be true. So you need to change the header file and make sure the static member is instantiated exactly once. If you use the header file you will potentially have multiple instantiations which will lead to linker errors. So again I find assertions to be wrong. \$\endgroup\$ – Martin York May 13 '18 at 19:33
2
\$\begingroup\$

First of all, you can now make inline static data members, which is designed specifically for being able to make header-only libraries without needing a CPP file just to hold the point of definitions.

Your main usability issue is the need to make a different instantiation of the template for each class you want to count.

Note that your counter can be a member as well as a base class. So don’t make it look like CRTP, because it’s not. The template instantiation does not need to know about the derived class it is a base of. It is just a unique label.

Here is another way to do that:

using counter1 = counter<struct tag1>;

That is, you can declare a struct type right in the template argument.
You can also:

using counter1 = counter<counter1>;

refer to the (incomplete) name being declared, so you don’t need yet another unique symbol name. This is similar to your inheriting use — the class being declared is just a handy type name to use.


You should make sure all the housekeeping members are provided, as the class you put this in will only get (x) if all its members and bases do (x). You forgot some. So did Martin in his answer. See §15 for the authoritative list.

This has been taught as “The Rule of Five” but that neglects that any defined destructor will also kill auto-generation of move functions, so it should really be the Rule of Six. Or for movie fans, Rule of 5.1 perhaps?


You are making the constructors private and the intended derived class a friend? Isn’t that what protected is for?


Another thought — if you want to avoid accidentally providing names that the derived class will inherit and step on something, you can make the accessor a free function template instead of a member function.

But what about the static counter itself? It still has a name. Is there a way to put that outside but still distinct for each instantiation?

There is indeed an easy way to do that. Make a variable template!

template<typename Tag>
atomic<uint32_t> count;

I don’t think this is what you are looking for, though, since you want to prevent other code from manipulating the counter. So make the template variable a class.

Actually, the template variable is just a shortcut for a static data member of a template, and now it moves inside a class like you are used to:

template <typename Owner>
class counters {
    static inline atomic<uint32_t> count { 0 };
    friend class count_this<Owner>;
public:
    static uint32_t get() { return count; }
};

Then the instrumentation class contains just the special members, no other members.

template <typename Owner>
struct count_this {
    count_this() { ++counters<Owner>::count }
      ⋮
\$\endgroup\$
  • \$\begingroup\$ I'm using C++14 so inline static members aren't available for me, but thanks for the tip. I really haven't done a good job at explaining my intentions with this class, so I edited my post. It explains the access specifiers and the use of crtp. \$\endgroup\$ – HenrikS May 13 '18 at 10:07
1
\$\begingroup\$
  • This class can be simplified: there is no need for nested counter class as well as for friend declarations if proper access specifiers are used: private for s_counter and protected for all the constructors / destructor.

  • This class would be more flexible of counter value type was supplied as a template parameter (with default value):

template<typename t_derived, typename t_counter = std::atomic<std::uint32_t>>
struct countable
  • All functions here could make use of noexcept specifier.

  • Putting more asserts could be helpful as well, for example at destructor:

~countable()
{
    assert(0 < counter::s_counter);
    --counter::s_counter;
}
\$\endgroup\$
  • \$\begingroup\$ Making s_counter templated is a good idea although I think it's not really necessary. I will add noexcept, always forget these :). Since s_counter can't be modified from outside I think additional asserts are unnecessary. I also edited my post to explain the access specifiers. Thanks for your feedback! \$\endgroup\$ – HenrikS May 13 '18 at 10:02
  • \$\begingroup\$ @HenrikS There is nothing preventing s_counter from being accidentally modified from outside or from countable destructor being called twice. So asserts are never redundant. I think it could be also a good idea to put an assert for maximum number of instances (and allow user to supply the max value). \$\endgroup\$ – VTT May 13 '18 at 19:30
  • \$\begingroup\$ Could you elaborate how s_counter can be modified from outside countable \$\endgroup\$ – HenrikS May 13 '18 at 19:50
  • \$\begingroup\$ @HenrikS (true story) someone accidentally overwrites bss section. Note that assertions are definitely not capable of making code completely bullet-proof, however they definitely help to detect and localize problems faster when unexpected things happen. \$\endgroup\$ – VTT May 13 '18 at 19:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.