Calling methods for all non-null objects in a pointer list

Asperamanca

Let's say I have a class with various methods.
In another class, I have a hash of pointers to it, like so:

QHash<int, MyClass*> m_MyClasses;

I have several places where I want to call a single method of MyClass for all instances.
At the moment, my code looks like this:

void callAllMethod1()
{
   for (MyClass* const pMyClass : m_MyClasses)
   {
      if (pMyClass == nullptr)
      {
         continue;
      }
      pMyClass.method1();
   }
}

void callAllMethod2()
{
   for (MyClass* const pMyClass : m_MyClasses)
   {
      if (pMyClass == nullptr)
      {
         continue;
      }
      pMyClass.method2();
   }
}

and so on,
Somehow I feel there must be a more elegant way to write this code, so that I can have the loop and nullptr-check only once, and just distinguish the method to call.

Sometimes I want to find if any of the methods called return "true". For this, I feel that std::anyOf should offer a better way, but I do not quite see how to incorporate the nullptr-check to avoid crashes.

Suggestions?

jsulm

@Asperamanca I'm wondering why you would keep null pointers in the hash? Why not just remove the entry from m_MyClasses if pointer is null (or do not insert null pointers in the first place)? Then there is no need to check the pointers all the time.

Konstantin Tokarev

Create a wrapper for std::mem_fn and use it with std::for_each

VRonin

use the functional library

// #include <functional>
    template <class Container, class Method>
    static void applyNotNull(Container& con, Method meth){
        auto endPoint=std::end(con);
        for(auto i=std::begin(con);i!=endPoint;++i){
           if(*i)
               std::bind(meth,*i)();
        }
    }
    template <class Container, class Method>
    static bool anyOffNotNull(Container& con, Method meth){
        auto endPoint=std::end(con);
        for(auto i=std::begin(con);i!=endPoint;++i){
           if(*i){
               if(std::bind(meth,*i)())
                   return true;
           }
        }
        return false;
    }

now you can use it with applyNotNull(m_MyClasses,&MyClass::method1) or applyNotNull(m_MyClasses,&MyClass::method2)

Asperamanca

@jsulm said in Calling methods for all non-null objects in a pointer list:

@Asperamanca I'm wondering why you would keep null pointers in the hash? Why not just remove the entry from m_MyClasses if pointer is null (or do not insert null pointers in the first place)? Then there is no need to check the pointers all the time.

It's not planned that there are null pointers in the Hash. However, I want to write code that is safe to execute even in this case (plus my static code analysis would give me a hard time if I didn't check it)

kshegunov

@Asperamanca said in Calling methods for all non-null objects in a pointer list:

Somehow I feel there must be a more elegant way to write this code, so that I can have the loop and nullptr-check only once, and just distinguish the method to call.

There is and Qt uses it extensively. The so called pointer-to-member types are what you're asking about:

typedef void (MyClass::*MyClassMethod)();
void callAll(MyClassMethod method)
{
   for (MyClass* const pMyClass : m_MyClasses)  {
      if (!pMyClass)
         continue;

      (pMyClass->*method)();
   }
}

And use like:

callAll(&MyClass::method1);
callAll(&MyClass::method2);

VRonin

@kshegunov said in Calling methods for all non-null objects in a pointer list:

typedef void (MyClass::*MyClassMethod)();

You really hate templates, don't you?! 😉

kshegunov

@VRonin said in Calling methods for all non-null objects in a pointer list:

@kshegunov said in Calling methods for all non-null objects in a pointer list:

typedef void (MyClass::*MyClassMethod)();

You really hate templates, don't you?! 😉

In a sense. I see no reason for endless and useless instantiation, yes. What I wrote is basically the same as what you wrote, with the exception of the call to std::bind you make - I just copied the original loop and tweaked it.

Asperamanca

Thank you for your suggestions. Based on that, I have made a wrapper:

class MyMethods
{
public:
    template <class IteratorT, class MethodT, class... Args>
    static void callForEach(const IteratorT& iterFrom,
                            const IteratorT& iterTo,
                            MethodT method,
                            Args... args)
    {
        for(auto iter=iterFrom;iter != iterTo;++iter)
        {
            if ((*iter) == nullptr)
            {
                continue;
            }

            method(*iter,args...);
        }
    }

    template <class ContainerT, class UnaryFunctionT, class... Args>
    static void callForEach(const ContainerT& container,
                            UnaryFunctionT method,
                            Args... args)
    {
        callForEach(container.constBegin(),
                    container.constEnd(),
                    method,
                    args...);
    }
}

Checks for nullptr, then calls a method with any number of arguments.
Usage:

MyMethods::callForEach(myContainer,
                       std::mem_fn(&MyClass::myMethod));

Next up:

Find a solution for a situation where I don't want to call a method to MyClass, but instead call a method of another class that takes the (checked) pointer of MyClass as first parameter
Create a wrapper so I can use things like std::any_of

Asperamanca

Wrapper for AnyOf:

    template <class IteratorT, class MethodT, class... Args>
    static bool anyOf(const IteratorT& iterFrom,
                      const IteratorT& iterTo,
                      MethodT method,
                      Args... args)
    {
        for(auto iter=iterFrom;iter != iterTo;++iter)
        {
            if ((*iter) == nullptr)
            {
                continue;
            }

            if (method(*iter,args...))
            {
                return true;
            }
        }

        return false;
    }

    template <class ContainerT, class MethodT, class... Args>
    static bool anyOf(const ContainerT& container,
                      MethodT method,
                      Args... args)
    {
        return anyOf(container.constBegin(),
                     container.constEnd(),
                     method,
                     args...);
    }

Further idea: Use type traits to make the methods work for containers with objects as well (e.g. QLIst<QRect>)