Improvement for std::map <int, run_some_function> double dispatch

Question

I am trying to do my c++ homework and as a part of it I have to implement a std::map with an int as its key and a value should be a specific void function, which prints some text. I can`t find an answer to what are the correct ways of doing that.

This is what I have so far:

#include <map>
#include <string>
#include <iostream>

class testClass
{
public:
    void testFunc1() { std::cout << "func1\n"; }
    void testFunc2() { std::cout << "func2\n"; }
};

typedef void(testClass::*runFunc)(void);
typedef std::map<int, runFunc> myMapType;

int main() {
    testClass t1;

    std::map<int, runFunc> myMap;
    myMap.emplace(1, &testClass::testFunc1);
    myMap.emplace(2, &testClass::testFunc2);

    myMapType::const_iterator itr;
    itr = myMap.find(2);

    if (itr != myMap.end()) {
        (t1.*(itr->second))();
    }
}

The code is working now, however I am not sure if that is the way people with more experience do.

Also, if you are aware of any sources where one can read more about knowledge required to achieve the needed result - any ideas are appreciated.

If your code is a simplified version of what you really have to do, then it's off-topic and this question needs to be closed; per codereview.stackexchange.com/help/on-topic - "In order to give good advice, we need to see real, concrete code, and understand the context in which the code is used. Generic code (such as code containing placeholders like foo, MyClass, or doSomething()) leaves too much to the imagination." — Reinderien
– Reinderien, Commented Dec 26, 2018 at 21:51

Quuxplusone · Accepted Answer · 2018-12-27 02:06:04Z

As other answers have already said, the trick you're looking for is called std::function, and it's defined in the standard <functional> header.

You include <string> but never use it. You define myMapType but never use it.

Unless you need portability back to C++03, you should use using X = Y; in place of typedef Y X;.

A common name for "scratch" iterator variables is it (not to be confused with i for integer indices). Your itr is reasonably clear, but not idiomatic.

Following the dictum that "good C++ code should look like Python," I'd write your program like this:

#include <cstdio>
#include <functional>
#include <map>

void testFunc1() {
    puts("func1");
}    
void testFunc2() {
    puts("func2");
}

using MapType = std::map<int, std::function<void()>>;

int main() {
    MapType myMap = {
        { 1, testFunc1 },
        { 2, testFunc2 },
    };

    auto it = myMap.find(2);
    if (it != myMap.end()) {
        it->second();
    }
}

Instead of the map initializer I wrote, you could write

MapType myMap;
myMap[1] = testFunc1;
myMap[2] = testFunc2;

but I strongly recommend "declarative" over "imperative." "Declarative" style means to define the whole map at once, as a data object, in the state you want it; "imperative" style means to define an empty map and then repeatedly mutate it so as to eventually arrive at the state you want. "Declarative" tends to be easier to reason about.

I could do the benchmarking myself, but does anyone know already if there is any overhead to using std::function<> rather than function pointers here? Also, is there any benefit to creating a map entry with a lambda [](){ testFunc(); } rather than with the function name directly? — SG_90
– SG_90, Commented Aug 12, 2022 at 13:52

πάντα ῥεῖ · Accepted Answer · 2018-12-26 15:05:48Z

In general your code looks good. But I have some little annotations (improvements) for you.

1. I'd prefer direct initialization of `itr` for sake of readability:

myMapType::const_iterator itr = myMap.find(2);

instead of

myMapType::const_iterator itr;
itr = myMap.find(2);

That said you could also avoid

typedef std::map<int, runFunc> myMapType;

completely using the auto keyword:

auto itr = myMap.find(2);

2. Make use `std::function` and lambda bindings

For the callable value parameter of the std::map I'd prefer to use an appropriate std::function value type and lambda function bindings to the instance to operate on.

testClass t1;

std::map<int, std::function<void()>> myMap;
myMap.emplace(1, [&t1](){t1.testFunc1();});
myMap.emplace(2, [&t1](){t1.testFunc2();});

This will give you greater flexibility for later changes and use of other classes than just testClass. Also you can get rid of that other type definition then:

typedef void(testClass::*runFunc)(void);

As soon you want to use your map for the general case you can clearly see the benefits:

class A {
public:
     void print() { std::cout << "print() from class A.\n"; }
};

class B {
public:
     void print() { std::cout << "print() from class B.\n"; }
};

// ...
int main() {
    A a;
    B b;

    std::map<int, std::function<void()>> myMap;
    myMap.emplace(1, [&a](){a.print();});
    myMap.emplace(2, [&b](){b.print();});

}

Here's my set of changes in whole (you can check it's still working as intended here):

#include <map>
#include <string>
#include <iostream>
#include <functional>

class testClass
{
public:
    void testFunc1() { std::cout << "func1\n"; }
    void testFunc2() { std::cout << "func2\n"; }
};

int main() {
    testClass t1;

    std::map<int, std::function<void()>> myMap;
    myMap.emplace(1, [&t1](){t1.testFunc1();});
    myMap.emplace(2, [&t1](){t1.testFunc2();});

    auto itr = myMap.find(2);

    if (itr != myMap.end()) {
        (itr->second)();
    }
}

Instead of a lambda that calls the function you can use std::bind. — Emily L.
– Emily L., Commented Dec 26, 2018 at 15:55
@EmilyL. But lambdas are preferred over std::bind! There is no need for it now. Lambdas are far more expressive. — Lightness Races in Orbit
– Lightness Races in Orbit, Commented Dec 27, 2018 at 2:33
@lightnessracesinorbit that's the first I've heard, can you give a reference? I still feel like bind is better expressing the intent — Emily L.
– Emily L., Commented Dec 27, 2018 at 12:24

Reinderien · Accepted Answer · 2018-12-26 17:54:03Z

0

Given your current test code, there's no reason for testClass to exist. Your two test functions can simply be functions in the global namespace, and then your emplace calls can be simplified.

void testFunc1() { std::cout << "func1\n"; }
void testFunc2() { std::cout << "func2\n"; }

typedef void(*runFunc)(void);
// ...

myMap.emplace(1, testFunc1);
myMap.emplace(2, testFunc2);

answered Dec 26, 2018 at 17:54

Reinderien

71.1k5 gold badges76 silver badges256 bronze badges

\$\begingroup\$ thanks for your suggestion. my code above is a simplified version of what I really have to do. with just global functions it won't go. \$\endgroup\$

Gasper J.
– Gasper J.

2018-12-26 18:40:51 +00:00
Commented Dec 26, 2018 at 18:40
\$\begingroup\$ See comments on question, in that case. This is off-topic. \$\endgroup\$

Reinderien
– Reinderien

2018-12-26 21:52:29 +00:00
Commented Dec 26, 2018 at 21:52

Add a comment |

Stack Exchange Network

Improvement for std::map <int, run_some_function> double dispatch

3 Answers 3

1. I'd prefer direct initialization of `itr` for sake of readability:

2. Make use `std::function` and lambda bindings

You must log in to answer this question.

Hot Network Questions

Improvement for std::map <int, run_some_function> double dispatch

3 Answers 3

1. I'd prefer direct initialization of itr for sake of readability:

2. Make use std::function and lambda bindings

You must log in to answer this question.

Related

Hot Network Questions

1. I'd prefer direct initialization of `itr` for sake of readability:

2. Make use `std::function` and lambda bindings