5
\$\begingroup\$

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.

\$\endgroup\$
  • 1
    \$\begingroup\$ 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." \$\endgroup\$ – Reinderien Dec 26 '18 at 21:51
2
\$\begingroup\$

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.

\$\endgroup\$
5
\$\begingroup\$

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)();
    }
}
\$\endgroup\$
  • \$\begingroup\$ Instead of a lambda that calls the function you can use std::bind. \$\endgroup\$ – Emily L. Dec 26 '18 at 15:55
  • 2
    \$\begingroup\$ @EmilyL. But lambdas are preferred over std::bind! There is no need for it now. Lambdas are far more expressive. \$\endgroup\$ – Lightness Races with Monica Dec 27 '18 at 2:33
  • \$\begingroup\$ @lightnessracesinorbit that's the first I've heard, can you give a reference? I still feel like bind is better expressing the intent \$\endgroup\$ – Emily L. Dec 27 '18 at 12:24
  • 2
    \$\begingroup\$ @EmilyL. Sure! stackoverflow.com/a/17545183/560648 \$\endgroup\$ – Lightness Races with Monica Dec 27 '18 at 14:02
0
\$\begingroup\$

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);
\$\endgroup\$
  • \$\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. Dec 26 '18 at 18:40
  • \$\begingroup\$ See comments on question, in that case. This is off-topic. \$\endgroup\$ – Reinderien Dec 26 '18 at 21:52

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.