2

I have a virtual class, named Type, and to derived classes, Type1 and Type2.

class Type {
public:
    virtual void print() = 0;
};
class Type1 : public Type {
public:
    void print() { cout << "I am of type 1" << endl; }
};
class Type2 : public Type {
public:
    void print() { cout << "I am of type 2" << endl; }
};

Depending on parameters the user will enter, I will instantiate one class or the other.

Then I want to have another class that will produce actions, depending on the type of the "Type" object given as parameter. For now, this class is :

class Action {
protected:
    Type *t;
public:
    Action(Type *t) : t(t) {} ;
    void print() {
        cout << "I am an action. My type says: ";
        t->print();
    }
};

How can I have Action::print produce different tasks depending on the type of the attribute "t"?

What I tried:

  • Create two derived classes, namely Action1 and Action2, and instanciate Action1 if I instantiate Type1 (resp. Action2 and Type2). But this is exactly what I would like not to do: I would like Action to know "by itself" what to do.
  • Define Action as a templated class (replacing Type by the template type). But in the same way, I have to instantiate an object of type Action< Type1> or Action< Type2>, what I would like not to do.

My wish is to have a main that looks like:

int main(int argc, const char * argv[]) {
    Type *t = new Type1();

    Action *act = new Action(t);
    act->print();

    delete act, t;
    return 0;
}

But with a different print method called depending on the type of t. Is this possible (and how) or not?


Edit

After reading the comments, it turns that there are two main possible designs:

  1. Create one class, Action, and call the specific method (print(Type1 *) or print(Type2 *)). I think this is the visitor pattern suggested in the comments.

  2. Create two classes, Action1 and Action2 (possibly derived from Action, or not!), and instantiate the one that corresponds to t, Type1 or Type2.

Is one solution cleaner, easier to maintain, etc., than the other?


Edit 2

What about a Factory pattern, mentioned in some comments? Can anyone enlighten me about it solves or not my problem?

Simpom
  • 938
  • 1
  • 6
  • 23
  • 1
    I don't see the problem... seems to me like it does exactly what you want, right? You put the functionality in Type1/Type2 and use polymorphism to differentiate the implementation; sounds okay to me... – atlaste Apr 12 '17 at 07:18
  • @atlaste: in fact I would like Action to have a different behavior depending on the type of t. – Simpom Apr 12 '17 at 07:22
  • 2
    The behavior is of course the behavior of the component along with all the child components. If you need different Action behaviors, you need different implementations. But perhaps you're looking for a Visitor pattern? https://en.wikipedia.org/wiki/Visitor_pattern – atlaste Apr 12 '17 at 07:23
  • I think the visitor pattern seems like a good match and I also think the title is asking a completely different question (factory?). – stefaanv Apr 12 '17 at 07:26
  • @stefaanv: it has been difficult for me to find a good title. If you can suggest me one, I will change it – Simpom Apr 12 '17 at 07:32
  • 1
    If the action depends on the concrete type of the object and not on its polymorphic behavior, you probably don't want to use inheritance here. You can work around the issues with inheritance based polymorphism with a Visitor as suggested before. But I think the cleaner way would be to not have a common base class at all and just use a Boost or c++17 variant. Than only works if you will at all times now all possible "instances". If this is part of a library where users might add their own derived classes, then you unfortunately can't use a variant. – Corristo Apr 12 '17 at 07:33
  • If I understand the visitor pattern, there is one class, with several functions, each one being called with a specific type of parameter. Is it possible to have different classes? – Simpom Apr 12 '17 at 07:43
  • @Simpom As a suggestion for a title, how about "How to perform different tasks depending on the type of a constructor parameter?" – Chris Drew Apr 12 '17 at 07:44
  • @ChrisDrew: modified! (thx) – Simpom Apr 12 '17 at 07:46
  • @Simpom That is still not right because you are not "instantiating different classes" you are always instantiating an `Action`. – Chris Drew Apr 12 '17 at 07:47
  • @ChrisDrew My wish is to instantiate different classes... (but I admit that is not obvious in my description) – Simpom Apr 12 '17 at 07:49
  • @Simpom. Ah, OK, I missunderstood you! Well the constructor of `Action` always creates an `Action` so what you want to do is impossible. – Chris Drew Apr 12 '17 at 07:51
  • There _is_ the possibility to use `dynamic_cast` to see what type of `Type` instance you got in the `Action` methods. But using a `dynamic_cast` is usually a major code smell – king_nak Apr 12 '17 at 08:02
  • @king_nak ah we had the same thought (see my answer). I don't really know if this satisfies the question since the formulation seems to be a bit unclear to me. – dtell Apr 12 '17 at 08:09
  • @king_nak,datell Thx for the suggestions (I did not know about dynamic_cast), but as pointed I think it is a code smell. – Simpom Apr 12 '17 at 08:13
  • I just checked the visitor pattern once again (and finally understood it...) I think that's the way to go here, as already suggested by @atlaste – king_nak Apr 12 '17 at 08:23
  • 1
    I want to ask why you can not put your "Action" into the Type class. If you do so, you simply use virtual methods. If you dislike adding all functionality to your Type class family, you can use the mixin technique to add this parts of functionality to your code. As you generate Type classes from user input (factory-pattern) you can also generate the types classes with added Action functionality with a different factory, which maybe uses a simple set of mixin add ons. I personal dislike the idea of looking of the type and do a manual switch. Also visitor pattern can fast grow in complexity – Klaus Apr 12 '17 at 08:29
  • @Klaus In fact my real Type and Action classes manage respectively the thermodynamics and the chemistry of a system: they are linked (what leads to my question), but it seems confusing to mix them. Concerning the second part of your comment ("the mixin technique"), I don't get what you mean. Could you explain? – Simpom Apr 12 '17 at 08:44
  • Look here: http://stackoverflow.com/questions/18773367/what-are-mixins-as-a-concept. The idea behind mixin is, that you can implement your final class as a set of fragments which are all totally independent. So you can have your chemistry and thermodynamics implemented in separate classes, but for simpler execution you "mix" them together. Technically, you will generate only one vtable for both class sets. That combines a technical cheap and simple solution with the OOP idea of separation of logical units. – Klaus Apr 12 '17 at 08:48
  • Regarding your edit: Having different `Action` classes for the different types (maybe derived from a a common base) doesn't make sense for the following reason: You always need to know which type you currently have. But if you already know which type it is at compile time, there is no need for inheritance in the first place - you can always pass the concrete types everywhere. And if you don't know the concrete type, you need a way to figure it out - which is what you're trying to do right now. – Corristo Apr 12 '17 at 09:37

5 Answers5

2

A small remark first: I personally consider 'using namespace' a bad practice, because it ripples through header files. The reason is described throughout the internet.

Also, I always attempt to minimize using pointers, because of exception safety. Sutter has a nice book about it, called Exceptional C++, which describes these problems in a lot of detail. However, pointers obviously have their uses and one in particular is polymorphism. Lastly, I like making classes struct's if they only have public members... which is just a matter of taste.

Let's start with some code:

#include <string>
#include <iostream>
#include <memory>

struct Type
{
    virtual void print() = 0;
};

struct Type1 : Type
{
    void print() { std::cout << "I am of type 1" << std::endl; }
};

struct Type2 : Type
{
    void print() { std::cout << "I am of type 2" << std::endl; }
};

class Action
{
protected:
    std::unique_ptr<Type> t;
public:
    Action(std::unique_ptr<Type> &&t) : t(std::move(t)) {};

    void print()
    {
        std::cout << "I am an action. My type says: ";
        t->print();
    }
};

int main()
{
    Action act(std::make_unique<Type1>());
    act.print();

    return 0;
}

The first problem you seem to address is the fact that user input generates the types. Depending on the input, this could point to an abstract factory or builder pattern or even a full-blown parser, but if it's an easy input decision, best to KISS:

int main()
{
    std::string s;
    std::getline(std::cin, s);

    std::unique_ptr<Type> type;

    if (s == "1") 
    { 
        type = std::make_unique<Type1>();
    }
    else
    {
        type = std::make_unique<Type2>();
    }

    Action act(std::move(type));
    act.print();

    return 0;
}

Usually you want to separate your model from the implementations though. Actions come in a lot of different shapes and forms, so you probably want to do something else depending on your model. A visitor pattern, double dispatch, or whatever you want to call it comes to the rescue here.

Note that I normally use a somewhat modified visitor, which returns the base type as a default. That way, you can easily incorporate transformations into your code, which imho is a common design requirement. At this point I'm just going to use pointers. Personally I'm a big fan of memory arena's to work around the issues of memory management, but I consider this out of scope for this answer.

To finish things off, a simple base class can help you get rid of the senseless plumbing.

struct TypeVisitor;

struct Type
{
    virtual Type* Accept(TypeVisitor& visitor) = 0;
};

template <typename T>
struct TypeBase : Type
{
    virtual Type* Accept(TypeVisitor& visitor) override 
    { 
        return visitor.Handle(static_cast<T*>(this)); 
    }
};

struct Type1 : TypeBase<Type1>
{
};

struct Type2 : TypeBase<Type2>
{
};

struct TypeVisitor
{
    virtual Type* Handle(Type1* input)
    {
        /* if necessary, recurse here like: input->child = input->child->Accept(this); */
        return input;
    }
    virtual Type* Handle(Type2* input) { return input; }
};


struct DumpAction : TypeVisitor
{
    virtual Type* Handle(Type1* input) override
    {
        std::cout << "Handling type 1." << std::endl;
        return TypeVisitor::Handle(input);
    }

    virtual Type* Handle(Type2* input) override
    {
        std::cout << "Handling type 2." << std::endl;
        return TypeVisitor::Handle(input);
    }
};

int main()
{
    DumpAction act;
    Type2 type2;
    type2.Accept(act);

    return 0;
}
atlaste
  • 30,418
  • 3
  • 57
  • 87
  • The `using namespace` is effectively here only because it is a single snippet, and won't be present in my final code. I continue reading to try and understand your answer (I'm seeing my limitations in both C++ and english...) – Simpom Apr 12 '17 at 12:43
  • Sure. And that's okay; C++ seems to have that effect on people... ;-) – atlaste Apr 12 '17 at 12:54
  • Do I understand correctly: the main key of your solution is to reverse the roles between Type and Action. Instead of giving the Type to use to the Action (what I tried), you tell to Type that it will be used by Action. Is that it? – Simpom Apr 12 '17 at 13:00
  • I guess you can see it like that. 'Type' and 'Action' might be confusing names here. I see the 'Type's here as part of a larger model. And the 'action' is actually a transformation or operation on the model. There's no real relation between the two, other than the fact that operations use the model. So, naturally, code ends up in the operation classes (e.g. the 'Action' things) and the data structure ends up in the model classes (e.g. the 'Type' things). For the same reason as the fact that 'multiply' isn't part of an 'int'. The rest is glue... The net result is less typing and cleaner code. – atlaste Apr 12 '17 at 13:08
1

Your code is working and of good design. As said in the comments "you put the functionality in Type1/Type2 and use polymorphism to differentiate the implementation" which is a good approach.
You can follow this approach and do the implementation in Type1/Type2 and control the ouput in Action this way.

Dynamic cast

Regardless point one you can achieve what you want by just checking the types. You want to check if t in the constructor of Action is of type Type1 or of type Type2. Simply try to downcast it using dynamic_cast<T>.

Your code for Action::print() could look similar to this:

Action::print(){
    Type1* t1 = dynamic_cast<Type1*>(t); /* returns nullptr if t is not of type Type1

    /* Check if we got the nullptr or not */
    if(t1){
        std::cout << "I am an action. My type says: Type 1!" << std::endl;
    }
    else{
         std::cout << "I am an action. My type says: Type 2!" << std::endl;
    }
}

Let's check in int main():

int main(){
    Type *t1 = new Type1();
    Action *act1 = new Action(t1);
    act1->print();

    Type *t2 = new Type2();
    Action *act2 = new Action(t2);
    act2->print();

    delete act1,act2,t1,t2;
}

Output:

I am an action. My type says: Type 1!
I am an action. My type says: Type 2!

Please not that this is a minimal example and that I think a solution using clear structured polymorphism is the better alternative.

dtell
  • 2,488
  • 1
  • 14
  • 29
1

While inheritance-based polymorphism does have its uses, this doesn't sound like one of them. Generally, in an inheritance-based design, the task performed should only depend on the behavior of the involved classes, not on their concrete type.

A way around that limitation of inheritance is to use the Visitor pattern, which essentially exposes the concrete type of a class in the inheritance hierarchy through how it reacts to a visitation request.

The one use-case where you can't get around a inheritance-based design with Visitor pattern is when users whose code-base you cannot control might add additional derived classes. But as far as I can tell this isn't a concern here.

So the other way to get this to work is through variant instead of inheritance (I'm using c++17 std::variants here, but you can do the same with Boost.Variant if you don't have access to a compiler/STL that already implements std::variant.) Just define your types as

class Type1 {
public:
    void print() const {
        std::cout << "I am of type 1\n"; 
    }
};

class Type2 {
public:
    void print() const { 
        std::cout << "I am of type 2\n"; 
    }
};

and define the "base type" with a using-declaration

using Type = std::variant<Type1, Type2>;

The action itself would look like this:

class Action {
    Type t_;
public:
    Action(Type t)
        : t_(std::move(t)) {
     }

     void print() {
         auto action = make_combined(
             [](Type1 const& t) { std::cout << "Called with argument of type Type1\n"; t.print(); },
             [](Type2 const& t) { std::cout << "Called with argument of type Type2\n"; t.print(); } 
         );
         std::visit(action, t_);
     }
 };

where we need a little boilerplate for the visitation to work

template<typename... Ts>
struct combined : Ts... {
    combined(Ts... ts)
        : Ts(ts)... {
    }
    using Ts::operator()...; 
};

template <typename... Ts>
auto make_combined(Ts&&... ts) {
    return combined<std::decay_t<Ts>...>{std::forward<Ts>(ts)...};
}

The calling code then looks like this:

int main() {
    Type t1 = Type1{}; 
    auto a1 = Action{t1};
    a1.print();

    auto a2 = Action{Type2{}};  
    a2.print();
}

The working example can be found on wandbox.

If you later would like to add an additional type Type3, just create it, add it to the variant in the Type using-declaration and the compiler will tell you where you need to add a new handler for the new type.

If you prefer some kind of default behavior for types not explicitly mentioned in the Action class, you can also just add a generic lambda to the combined lambda in the Action class like:

auto action = make_combined(
    [](Type1 const& t) { std::cout << "Called with argument of type Type1\n"; t.print(); },
    [](Type2 const& t) { std::cout << "Called with argument of type Type2\n"; t.print(); },
    [](auto const& t) { std::cout << t.print(); } // this will be selected for all other types 
);
Corristo
  • 4,911
  • 1
  • 20
  • 36
1

Using mixing you can write your classes very natural. You only extend the CRTP pattern to make them able to use mixin technique later. The code is simpler than any textual explanation, so please look yourself:

#include <iostream>
#include <memory>

class Type
{
    public:
        virtual void Do()=0;
};

template <typename Base>
class Type1: public Base
{
    void Do() override { std::cout << "Type1::Do" << std::endl; }
};

template <typename Base>
class Type2: public Base
{
    void Do() override { std::cout << "Type2::Do" << std::endl; }
};

template <typename Base>
class Action: public Base
{
    public:
    virtual void Print()=0;
};

template <typename Base>
class Action1: public Base
{
    void Print() override { std::cout << "Print for Action1" << std::endl;}
};

template <typename Base>
class Action2: public Base
{
    void Print() override { std::cout << "Print for Action2" << std::endl;}
};

int main()
{

    // what we want "mixin" is Action into Type
    using Base = Action<Type>;

    std::unique_ptr<Base> a1 { new Action1<Type1<Base>> };    // Action1 in Type1
    std::unique_ptr<Base> a2 { new Action2<Type2<Base>> };    // Action2 in Type2

    a1->Do();
    a1->Print();

    a2->Do();
    a2->Print();
}
Klaus
  • 24,205
  • 7
  • 58
  • 113
  • You're leaking memory now, why not use a `std::unique_ptr` instead of raw pointers? – Corristo Apr 12 '17 at 09:09
  • I don't think that solves Simpoms problem, as you need to know the concrete type before creating the correct action. So this doesn't really allow for runtime polymorphism, because you need a way to figure out the concrete type again - the exact problem Simpom was trying to solve in the first place. – Corristo Apr 12 '17 at 09:40
  • @Corristo: My understanding of the question is, that we have a constant relation between types and actions so this can be "hard coded". If we need multiple sets of action/type relations, we can switch the factory which generate the action/type mixins. I see no need for having runtime dynamic switching of actions to types. This was not requested. Your "make_combined" table also hard codes this relation. Maybe I misunderstood your comment. Generating the concrete type is part of a factory which is not shown in my example. But coding a factory is well described in the net. – Klaus Apr 12 '17 at 09:46
  • Well I don't catch everything in the code you posted, but what I see is that you have to declare that a1 is of type Action1>, while I wish it could be Action. – Simpom Apr 12 '17 at 09:54
  • The point is you might want to write a function `auto foo(Type const& t) { return Action{t}; }` where you don't get a `Action1>` object but simply an instance of `Type`. How do you know which action to create? – Corristo Apr 12 '17 at 09:56
  • @Corristo: You simply can change Action class to ActionCreator class where a "CreateAction" function returns a concrete Action_x if you need a full isolated action type here. – Klaus Apr 12 '17 at 10:01
  • That works, but then you have the same problem that Simpom currently has: The `CreateAction` function needs a way to figure out the concrete type from the argument passed to it. – Corristo Apr 12 '17 at 10:03
  • @Corristo: ??? If CreateAction1 is used in the mixin for Type1 it will create a Action1. Can you explain what I have overseen? I can't catch your point, sorry. Maybe I am totally wrong and did not understand what he wants to achieve... You have not use "Type1" as argument for "CreateAction", you have to use Type1_instance_ptr->CreateAction(). Maybe that is the problem? This makes the mixin itself a Action-Factory! – Klaus Apr 12 '17 at 10:05
  • Assume there is a function `Type* getType();`. Now, somewhere later in the data flow I figure out I need to perform a certain action, so I'd like to create the corresponding `Action` object. If we try to encapsulate that in its own function it would look something like `std::unique_ptr getAction(Type* obj) { return ActionCreator{}.CreateAction(obj); }` How does `CreateAction` know which one to create? I think you assume that the original `getType()` function aready returns an object with the action mixed in, or a class derived from `Type` knows how to create all kinds of actions. – Corristo Apr 12 '17 at 10:13
  • @Corristo: Again: Do not use TypeX as parameter. Use TypeX_Insance_Ptr->CreateAction. Simply that! I will code it. Please stand by :-) – Klaus Apr 12 '17 at 10:17
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/141533/discussion-between-corristo-and-klaus). – Corristo Apr 12 '17 at 10:21
0

Here is what I came to.

#include <iostream>
using namespace std;

/** Class Type and its derived classes Type1 and Type2
 */
class Type {
public:
    int type;
    virtual void print() = 0;
};
class Type1 : public Type {
public:
    Type1() { type = 1; }
    void print() { cout << "I am of type 1" << endl; }
};
class Type2 : public Type {
public:
    Type2() { type = 2; }
    void print() { cout << "I am of type 2" << endl; }
};

/** Class Action and its derived classes Action1 and Action2
 */
class Action {
protected:
    Type *t;
public:
    Action(Type *t) : t(t) {};
    static Action *make_action(Type *t);
    virtual void print() = 0;
};
class Action1 : public Action {
public:
    Action1(Type *t) : Action(t) {};
    void print() {
        cout << "I am an action1. My type says: ";
        t->print();
    }
};
class Action2 : public Action {
public:
    Action2(Type *t) : Action(t) {};
    void print() {
        cout << "I am an action2. My type says: ";
        t->print();
    }
};

Action *Action::make_action(Type *t) {
    switch(t->type) {
    case 1:
        return new Action1(t);
        break;
    case 2:
        return new Action2(t);
        break;
    }
}

/** Main
 */
int main(int argc, const char * argv[]) {
    Type *ta = new Type1();
    Action *acta = Action::make_action(ta);
    acta->print();
    delete acta, ta;

    Type *tb = new Type2();
    Action *actb = Action::make_action(tb);
    actb->print();
    delete actb, tb;

    return 0;
}

The output is:

I am an action1. My type says: I am of type 1
I am an action2. My type says: I am of type 2

It seems like it does the expected job. If anyone sees a mistake, please let me know.

Simpom
  • 938
  • 1
  • 6
  • 23
  • Yeah I see plenty of "mistakes". :-) For example, your code will leak when print throws. Dealing with pointers in C++ is quite tricky... – atlaste Apr 12 '17 at 12:01
  • I think the separate `Action1` and `Action2` classes are unnecessary. Just move the `switch(t->type) ...` to `Action::print` and the respective implementation of `Action{1,2}::print` into the correct cases. Then all the different kind of actions will be in one place, so you quickly can figure out what is happening for a particular type. Otherwise you'll first need to look at the `make_action` function to figure out which concrete Action object is created, and then have a look at the implementation there. – Corristo Apr 12 '17 at 12:15
  • Another concern I have is that you might accidentally have two classes derived from `Type` with the same ID. If not all of them are defined in a single file, it might be hard to figure out which IDs are already taken when you're creating a new derived class. That's why the Visitor pattern defines different function names for the different derived classes to call. There they all are defined in the abstract Visitor base class. – Corristo Apr 12 '17 at 12:17
  • 1
    @Corristo For the two Action* classes, *my* feeling is the exact opposite: knowing the problem comes in a specific situation, I believ it is easier to go and see the class that deals with this situation. Concerning the type id, I thought about this problem but did not found a simple way to avoid it. – Simpom Apr 12 '17 at 12:40