Problem with filling a QMap with pointers to member functions

Question

I have a factory class supposed to construct objects of classes derived from a base class. The concrete instance is identified by a QString. I want to use a QMap (or similar) to map the QString to a member function pointer, but i get an error.

I tried various syntax variants, most of them found somewhere here in this forum.

    #include "messagefactory.h"

    #include "alivecheckmessage.h"

    namespace MES {

        QMap<QString, Message (MessageFactory::*)(void)>  MessageFactory::messageMap;

        MessageFactory::MessageFactory()
        {
            // Nothing, is private
        }

        Message MessageFactory::createAliveCheckMessage() {
            AliveCheckMessage msg;
            return msg;
        }

        Message MessageFactory::createMessage(QString id) {
            if (messageMap.isEmpty()) {
                messageMap[AliveCheckMessage::ID] = &createAliveCheckMessage;

            }
        }


    }

The error is in the last line. The error message says:

error: assigning to 'MES::Message (MES::MessageFactory::*)()' from incompatible type 'MES::Message (\*)()'

How am i supposed to correct that?

`messageMap[AliveCheckMessage::ID] = &MessageFactory::createAliveCheckMessage;` IIRC — john, Aug 19 '19 at 06:30
At a guess `createAliveCheckMessage` is static but we'd need a [mre] to be able to help — Alan Birtles, Aug 19 '19 at 06:48
Not sure whether you really want to write all these `create` member functions. There is a simpler way (with a generic create function) like shown in my answer to [SO: How can I improve my simple factory pattern?](https://stackoverflow.com/a/57505653/7478597). — Scheff's Cat, Aug 19 '19 at 06:52
Yes, both methods are static. And that AliveCheckMessage is just a class. You might think of it as simple empty class. — Siegfried, Aug 19 '19 at 06:53
This could be, of course, a template member function as well. (I would be afraid about all the scope and member function pointer call fiddling but, actually, I already did this - and would use my old code for cheating.) ;-) — Scheff's Cat, Aug 19 '19 at 06:57
@Scheff: The template is indeed nice. But it does not solve the problem here. Edit: Ok, maybe it would. I'm just not sure what it would mean to insert a template into the map. — Siegfried, Aug 19 '19 at 07:02
off-topic? This is not asking for debugging help, it is a syntax question. The question was understood and answered, although needed some minor discussing. So it's not off-topic, but answered and may be closed now. I'd do it myself, but i don't know how. — Siegfried, Aug 20 '19 at 08:00

score 1 · Answer 1 · answered Aug 19 '19 at 06:58

1

A static method is not the same type as a member function. Static methods are basically exactly the same type as a global function. You should either make your method non-static or change your map type to plain function pointers:

std::map<std::string, Message (*)(void)>

You also need to read up on What is object slicing? and change your methods to return pointers (preferably smart pointers) to Message.

answered Aug 19 '19 at 06:58

Alan Birtles

32,622
4
31
60

I tried this, too. That give 2 new errors: error: expected expression error: expected '(' for function-style cast or type construction. – Siegfried Aug 19 '19 at 07:14
@Siegfried It would help if you confirmed whether `createAliveCheckMessage` is or isn't static. The two answers you've been given are correct (for the static and non-static cases), so if neither is working then you are making some other error. But without seeing complete code we're only guessing what that might be. – john Aug 19 '19 at 07:19
But at least changing the methods to non-static solved the issue! Thanks. So i have to use a singleton instance. Well,... – Siegfried Aug 19 '19 at 07:23
@Siegfried No you dont have to use a singelton. But obviously to call a non-static member function you need an instance of some sort. I think you're making the common newbie assumption that a pointer to a member function stores the instance as well as the pointer to the function, it doesn't it's just a pointer. It's up to use to supply the instance one way or another. – john Aug 19 '19 at 07:38
Yes. I wanted to make it static for i do not wanted to have numerous instances of this factory class. For other reasons though. Although you're right, the code is not stored n-times per n instances :) – Siegfried Aug 19 '19 at 07:42
If this doesn't solve your issue then we need a [mre] to see the actual code you are using, we can't help to fix code we can't see – Alan Birtles Aug 19 '19 at 08:13
It seems to be solved. See comment below next answer. And yes, i also changed the methods to return socalled smart pointers :) – Siegfried Aug 19 '19 at 11:58

score 0 · Answer 2 · answered Aug 19 '19 at 06:40

0

Use the full qualified name of your factory class when assigning to the map:

messageMap[AliveCheckMessage::ID] = &MessageFactory::createAliveCheckMessage;

answered Aug 19 '19 at 06:40

Dumbo

13,555
54
184
288

I already tried that. Same result. And i tried it with and without the '&' operator. – Siegfried Aug 19 '19 at 06:46
To sum it up: The solution is a combination of Saeid Yazdanis answer and the answer from john. 1. It is necessary to use non-static member functions. 2. It is necessary to use the fully qualified method name. – Siegfried Aug 19 '19 at 07:39

Problem with filling a QMap with pointers to member functions

2 Answers2