3

This question stems from a previous question I asked here. I cannot use any external libraries or the C++ 11 spec. Meaning I can't use std::bind, std::function, boost::bind,boost::function etc. I have to write it myself. The issue is the following:

Consider the code:

EDIT

Here is a complete program that exhibits the problem as requested:

#include <map>
#include <iostream>


class Command {    
public:
    virtual void executeCommand() = 0;
};

class Functor {
public:
    virtual Command * operator()()=0; 
};

template <class T> class Function : public Functor {
private:
    Command * (T::*fptr); 
    T* obj;                  
public:
    Function(T* obj, Command * (T::*fptr)()):obj(obj),
        fptr(fptr) {}

    virtual Command * operator()(){
        (*obj.*fptr)();   
    }            
};

class Addition:public Command {
public:
    virtual void executeCommand(){
        int x;
        int y;
        x + y;
    }
};

class CommandFactory {
public:
    virtual Addition * createAdditionCommand() = 0;
};

class StackCommandFactory: public CommandFactory {
private:
    Addition * add;
public:
    StackCommandFactory():add(new Addition()) {}

    virtual Addition * createAdditionCommand(){
         return add;
    }
};

void Foo(CommandFactory & fact) {
    Function<CommandFactory> bar(&fact,&CommandFactory::createAdditionCommand);
}

int main() {
    StackCommandFactory fact;
    Foo(fact);
    return 0;
}

The error it gives is "no instance of constructor "Function<T>::Function [with T=CommandFactory] matches the argument list, argument types are: (CommandFactory *, Addition * (CommandFactory::*)())

I think it's complaining because I'm passing it a derived type. I have to use pointers/references to the abstract classes because fact may not be a StackCommandFactory later down the road.

I can't say:

void Foo(CommandFactory & fact){
      Function<CommandFactory> spf(&fact,&fact.createAdditionCommand); //error C2276

}

because of then I receive error C2276 which says (as in the question I linked to) '&' : illegal operation on bound member function expression.

So explicitly my question is: "How do I initialize this functor object so that I can use it with the above mentioned interfaces?"

Community
  • 1
  • 1
thed0ctor
  • 1,350
  • 4
  • 17
  • 34
  • Can you make a small **complete** program that exhibits this problem? Debugging un-seeable code is *far* more difficult than reducing the code to something we can all see. – Drew Dormann Mar 31 '13 at 19:21
  • Your `Function` template class should be header only, as `Foo()` is the only place where there is enough information to instantiate the template. This is the reason for the linker error. – quamrana Mar 31 '13 at 19:27
  • Identifiers which begin with an underscore and are followed by an uppercase letter are reserved, i.e., `_FUNCTOR_H_` should be `FUNCTOR_H_` (or just `FUNCTOR_H`). Actually, in the global namespace, any identifier beginning with an underscore is reserved. – Ed S. Mar 31 '13 at 19:31
  • @DrewDormann I made a small complete program as requested that exhibits the problem. quamrana I haven't had a linking error yet but will keep this in mind later thanks! Ed I didn't know that, I'll keep this in mind. – thed0ctor Mar 31 '13 at 19:46
  • What is `Add` in the error? I see the class `Addition` but not `Add` anywhere else in your code ... – maditya Mar 31 '13 at 20:09
  • That was a typo. I just corrected it – thed0ctor Mar 31 '13 at 20:18

3 Answers3

1

You need an explicit cast on the 2nd parameter of the bar instance:

Function<CommandFactory> bar(&fact,
  reinterpretet_cast<Command *(CommandFactory::*)()>(&CommandFactory::createAdditionCommand));

Besides, you're missing parens for the method pointer attribute in Function:

Command * (T::*fptr)();

This error might have prevented you to find the solution above.

You are also missing the return keyword in the operator() there (a mistake that I often do because of my functional programming habits):

virtual Command * operator()(){
   return  (obj->*fptr)();
}            

You can avoid the cast by making the return type a template parameter:

template <class T, typename D> 
class Function : public Functor {
private:
    D * (T::*fptr); 
    T* obj;
public:
    Function(T* obj, D * (T::*fptr)()): obj(obj),  fptr(fptr){}
    virtual Command * operator()(){
        return (obj->*fptr)();
    }            
};

void Foo(CommandFactory & fact){
    Function<CommandFactory, Addition> bar(&fact, &CommandFactory::createAdditionCommand);
}

Note that I did not templatize Functor after all. While it seemed a good idea at first to me, it make things a bit more complex. If you wish to make Functor a template too, the return type will have to be exactly the same, you cannot use an inheritance relation between them, unless you make them both parameters of the Function template. As a rule of thumb, whenever you stumble on a template issue like that, remember that template are like C macros at the core, it's a rewriting mechanism, which will expand the template into real C++ types (functions or classes) separately. You can picture the problem that way:

template <typename T, typename D>
class Function : public Functor<D> { /* ... */ };

will be expanded to

class Function<CommandFactory, Addition> : public Functor<Addition> {
    /* ... */
};

Functor<Addition> and Functor<Command> bears no relationship at all; these are two different classes.

If C++ template did carry the notion of bounded polymorphism (like in Java or C#), it could have perhaps been possible to write it in way close to your intent.

I recommend:

  • keeping the Functor a simple class, to make the code simpler to work with for the time being, and
  • if the need arises later on, trying to refactor a working version with that new feature.
didierc
  • 14,572
  • 3
  • 32
  • 52
  • I tried with my own experiment, which wasn't exactly as yours actually. You prompted me to verify it, and I saw my mistake. I hope this time my assessment of the problem is correct. – didierc Mar 31 '13 at 21:18
  • Currently it works in the test program so now I'm trying to figure out why it's throwing a `- pure virtual function call` exception. It's like it's not finding the right method. It calls the right one in the test program though – thed0ctor Mar 31 '13 at 22:31
  • The only issue with using multiple types in the template is that when I try to use a map of Functor objects, I get the same casting issue `Function bar(&fact,&CommandFactory::createAdditionCommand); std::map *> the_map; the_map["+"] = &Function (&fact,&CommandFactory::createAdditionCommand);` – thed0ctor Apr 01 '13 at 04:43
1

Here's a modification of my original answer that seems to do what you need, without using the any functor stuff from C++11 or boost.

#include <vector>
#include <map>
#include <string>

struct Command {};
struct Subtract : Command {};
struct Add : Command {};

class CommandFactory
{
  public:

    virtual Subtract * createSubtractionCommand() = 0;
    virtual Add * createAdditionCommand() = 0;
};

class StackCommandFactory : public CommandFactory
{
  public:

    virtual Subtract * createSubtractionCommand(void);
    virtual Add * createAdditionCommand(void);

    Subtract * sub;
    Add * add;
};

Subtract * StackCommandFactory::createSubtractionCommand(void) { return sub; }
Add * StackCommandFactory::createAdditionCommand(void) { return add; }

class CommandGetterImpl
{
  public:
    virtual CommandGetterImpl* clone() const=0;
    virtual Command* get()=0;
    virtual ~CommandGetterImpl() {};
};

class CommandGetter
{
  public:
  Command* get() { return impl_->get(); }
  ~CommandGetter() { delete impl_; }
  CommandGetter( const CommandGetter & other ) : impl_(other.impl_?other.impl_->clone():NULL) {}
  CommandGetter& operator=( const CommandGetter & other ) {
     if (&other!=this) impl_= other.impl_?other.impl_->clone():NULL;
     return *this;
  }
  CommandGetter() : impl_(NULL) {}
  CommandGetter( CommandGetterImpl * impl ) : impl_(impl) {}
  CommandGetterImpl * impl_;
};

class Parser
{
  public:
    Parser (CommandFactory & fact);

    std::map<std::string, CommandGetter > operations; 
};

template<typename MEMFN, typename OBJ >
class MemFnCommandGetterImpl : public CommandGetterImpl
{
  public:
  MemFnCommandGetterImpl(MEMFN memfn, OBJ *obj) : memfn_(memfn), obj_(obj) {}
  MemFnCommandGetterImpl* clone() const { return new MemFnCommandGetterImpl( memfn_, obj_) ; }
  Command* get() { return (obj_->*memfn_)(); }
  MEMFN memfn_;
  OBJ * obj_;
};


template< typename MEMFN, typename OBJ >
CommandGetter my_bind( MEMFN memfn, OBJ * obj )
{
  return CommandGetter( new MemFnCommandGetterImpl<MEMFN,OBJ>(memfn,obj) );
};

Parser::Parser(CommandFactory & fact)
{
  operations["+"] = my_bind(&CommandFactory::createAdditionCommand, &fact);
  operations["-"] = my_bind(&CommandFactory::createSubtractionCommand, &fact);
}

#include <iostream>
int main()
{
  Add add;
  Subtract sub;

  StackCommandFactory command_factory;
  command_factory.add = &add;
  command_factory.sub= &sub;
  Parser parser(command_factory);

  std::cout<<"&add = "<<&add<<std::endl;
  std::cout<<"Add = " <<  parser.operations["+"].get() <<std::endl;
  std::cout<<"&sub = "<<&sub<<std::endl;
  std::cout<<"Sub = " <<  parser.operations["-"].get() <<std::endl;

  return 0;
}
Michael Anderson
  • 70,661
  • 7
  • 134
  • 187
  • Well it took me three days and a conversation with my professor to fully understand why what I was doing wasn't working and to understand what you wrote above. Thanks so much for your help! – thed0ctor Apr 07 '13 at 02:39
-3

Generally speaking, it's a bad idea to use member function pointers as opposed to std::function. More generally,

typedef std::function<void()> Command;
typedef std::function<Command()> Functor;

Really, there's absolutely no need whatsoever for any member function pointers in your code.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • I guess the reason for down-votes is that he mentioned that "I cannot use any external libraries or the C++ 11 spec. Meaning I can't use std::bind, std::function, boost::bind,boost::function etc. I have to write it myself." – SChepurin Apr 02 '13 at 09:13