Best way to resolve this ambiguity?

What is the best way to resolve this ambiguity, while making as little change to the pattern used as possible, especially in main? The SingingAthlete shall put on the running shoes and sing with the microphone.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
#include <iostream>
#include <list>

struct Athlete;
struct Singer;

struct Object {
	virtual void accept (Athlete* a) = 0;
	virtual void accept (Singer* s) = 0;
};

struct RunningShoes: public Object {
	virtual void accept (Athlete* a) override {std::cout << "Athlete puts on the running shoes." << std::endl;}
	virtual void accept (Singer* s) override {std::cout << "Singer has no need for the running shoes." << std::endl;}
};

struct Microphone: public Object {
	virtual void accept (Athlete* a) override {std::cout << "Athlete has no need for the microphone." << std::endl;}
	virtual void accept (Singer* s) override {std::cout << "Singer sings with the microphone." << std::endl;}
};

struct Person {
	virtual ~Person() {}
	virtual void visit (Object* o) = 0;
};

struct Athlete: public virtual Person {
	virtual void visit (Object* o) {o->accept(this);}	
};

struct Singer: public virtual Person {
	virtual void visit (Object* o) {o->accept(this);}	
};

struct SingingAthlete: public Athlete, public Singer {
	virtual void visit (Object* o) {o->accept(this);}		
};

int main() {
	RunningShoes* shoes = new RunningShoes;
	Microphone* mic = new Microphone;
	std::list<Person*> people = {new Athlete, new Singer, new SingingAthlete};
	for (Person* x: people)
		{x->visit (shoes);  x->visit (mic);}
	std::cin.get();
}


I've had Singer and Athlete for quite some weeks now, and added a lot of code. Just today I added SingingAthlete (which I never foresaw doing) and ran into this ambiguity. So hopefully whatever correction can keep everything else intact. But I am willing to rework it all if the change is not too much.
Last edited on
Oh, the dreaded diamond inheritance. One tries his/her best to avoid such a calamity.

One way to solve it is to use virtual inheritance:
1
2
3
4
5
struct Athlete : public virtual Person{
//...

struct Singer : public virtual Person{
//... 


This ensures that AthleteSinger only has one Person object. Although, if you want to call overridden methods from Athlete or Singer, you'll still probably need to be explicit.

http://www.cprogramming.com/tutorial/virtual_inheritance.html

Edit:
Some links, so you can find out when best to use composition versus multiple inheritance.
http://stackoverflow.com/questions/406081/why-should-i-avoid-multiple-inheritance-in-c
http://www.parashift.com/c++-faq/mi-disciplines.html
http://stackoverflow.com/questions/573913/a-use-for-multiple-inheritance
http://stackoverflow.com/questions/49002/prefer-composition-over-inheritance
Last edited on
No that still doesn't work here, if you read my code. A SingingAthlete visiting a RunningShoe still would give two possible outputs. Hence the flaw in my code.

But I figured it out. Perhaps this is what you meant by being specific.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
#include <iostream>
#include <list>

struct RunningShoes;
struct Microphone;

struct Person {
	virtual ~Person() {}
	virtual void visit (RunningShoes* r) = 0;
	virtual void visit (Microphone* m) = 0;
};

struct Athlete: public virtual Person {
	virtual void visit (RunningShoes* r) {std::cout << "Athlete puts on the running shoes." << std::endl;}
	virtual void visit (Microphone* m) {std::cout << "Athlete has no need for the microphone." << std::endl;}
};

struct Singer: public virtual Person {
	virtual void visit (RunningShoes* r) {std::cout << "Singer has no need for the running shoes." << std::endl;}
	virtual void visit (Microphone* m) {std::cout << "Singer sings with the microphone." << std::endl;}
};

struct SingingAthlete: public Athlete, public Singer {
	virtual void visit (RunningShoes* r) {std::cout << "Singing athlete puts on the running shoes." << std::endl;}
	virtual void visit (Microphone* m) {std::cout << "Singing athlete sings with the microphone." << std::endl;}		
};

struct Object {
	virtual void accept (Person* person) = 0;
};

struct RunningShoes: public Object {
	virtual void accept (Person* person) {person->visit (this);}
};

struct Microphone: public Object {
	virtual void accept (Person* person) {person->visit (this);}
};

int main() {
	RunningShoes* shoes = new RunningShoes;
	Microphone* mic = new Microphone;
	std::list<Person*> people = {new Athlete, new Singer, new SingingAthlete};
	for (Person* x: people)
		{x->visit (shoes);  x->visit (mic);}
	std::cin.get();
}


Output:
Athlete puts on the running shoes.
Athlete has no need for the microphone.
Singer has no need for the running shoes.
Singer sings with the microphone.
Singing athlete puts on the running shoes.
Singing athlete sings with the microphone.
Last edited on
> Oh, the dreaded diamond inheritance. One tries his/her best to avoid such a calamity.

There is nothing to be scared of; there is no calamity.
Or else any programmer using std::cout would have kept running into insurmountable programming problems.

A SingingAthlete is an Athlete, is a Singer and both Athlete and Singer are Persons. There is no better way to express that in code other than multiple inheritance with a virtual base class at the root.



> One way to solve it is to use virtual inheritance:

Multiple inheritance, virtual inheritance, diamond(!) inheritance which petrifies people - all these are completely irrelevant to the problem at hand.


> I've had Singer and Athlete for quite some weeks now, and added a lot of code.
> Just today I added SingingAthlete (which I never foresaw doing) and ran into this ambiguity

The classic Go4 visitor pattern creates a cyclic dependency. To be able to freely add new derived classes (of Person in this case), an acyclic form of the visitor patter should be used.

This is one way to implement an acyclic visitor:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
#include <iostream>
#include <list>

struct Person { 
    struct visitor { virtual ~visitor() = default ; };
    virtual ~Person() = default ;
	virtual void accept( visitor* ) = 0;
};

struct Athlete : public virtual Person {
    
    struct visitor 
    {
        virtual ~visitor() = default ;
        virtual void visit( Athlete* ) = 0 ;
    };
    
	virtual void accept( Person::visitor* ov ) override 
    {
        auto av = dynamic_cast<Athlete::visitor*>(ov) ;
        if(av) av->visit(this) ; 
    }
};

struct Singer : public virtual Person {
    
    struct visitor 
    {
        virtual ~visitor() = default ;
        virtual void visit( Singer* ) = 0 ;
    };
    
    virtual void accept( Person::visitor* ov ) override 
    {
        auto sv = dynamic_cast<Singer::visitor*>(ov) ;
        if(sv) sv->visit(this) ; 
    }
};

struct SingingAthlete : public Athlete, public Singer {
    
    struct visitor 
    {
        virtual ~visitor() = default ;
        virtual void visit( SingingAthlete* ) = 0 ;
    };
    
    virtual void accept( Person::visitor* ov ) override 
    {
        auto sav = dynamic_cast<SingingAthlete::visitor*>(ov) ;
        
        if(sav) sav->visit(this) ; // if visitor implements SingingAthlete::visitor
        
        else // visit both Athlete::visitor and Singer::visitor
        {
            Athlete::accept(ov) ;
            Singer::accept(ov) ;
        }
    }
};

int main() {
    
    struct RunningShoes : Person::visitor, Athlete::visitor, Singer::visitor {
        virtual void visit( Athlete* ) override 
       { std::cout << "Athlete puts on the running shoes.\n" ; }
    	virtual void visit( Singer* ) override 
       { std::cout << "Singer has no need for the running shoes.\n" ; }
    };
    
    struct Microphone: Person::visitor, Athlete::visitor, Singer::visitor {
    	virtual void visit( Athlete*) override 
       { std::cout << "Athlete has no need for the microphone.\n" ; }
    	virtual void visit( Singer* ) override 
       { std::cout << "Singer sings with the microphone.\n" ; }
    };
    
    struct KaraokeWalkman : Person::visitor, SingingAthlete::visitor {
        virtual void visit( SingingAthlete* ) override 
       { std::cout << "SingingAthlete jogs with the KaraokeWalkman.\n" ; }
    };

    Person::visitor* objects[] = { new RunningShoes,  new Microphone, new KaraokeWalkman };
    std::list<Person*> people = { new Athlete, new Singer, new SingingAthlete };

    for( Person* x: people )
    {
        for( auto object : objects ) x->accept(object) ;
        std::cout << '\n' ;
    }
}

http://coliru.stacked-crooked.com/a/83991cba9544747d
Wow. Never knew about acylic visitor pattern. But the output:
1
2
3
4
5
6
7
8
9
10
11
Athlete puts on the running shoes.
Athlete has no need for the microphone.

Singer has no need for the running shoes.
Singer sings with the microphone.

Athlete puts on the running shoes.
Singer has no need for the running shoes.  // but SingingAthlete does need running shoes
Athlete has no need for the microphone.  // but SingingAthlete does need a microphone
Singer sings with the microphone.
SingingAthlete jogs with the KaraokeWalkman.


is supposed to be
1
2
3
4
5
6
7
8
9
Athlete puts on the running shoes.
Athlete has no need for the microphone.

Singer has no need for the running shoes.
Singer sings with the microphone.

Athlete puts on the running shoes.
Singer sings with the microphone.
SingingAthlete jogs with the KaraokeWalkman.


My second post gives the output I want, but will require hours of revising my program, and has the main drawback of changing the data structures (bad move!). Your solution has the damage control by only adding to the existing data structures, but it I still need the correct prototype before I implement it. I will study it though and perhaps figure out the correction myself.
Last edited on
One key idea behind this implementation of the the acyclic visitor is that it allows selective visitation.
A visitor can just ignore types that hold no interest to the visitor.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
int main() {
    
    // Singer has no need for RunningShoes, so we don't implement Singer::visitor 
    struct RunningShoes : Person::visitor, Athlete::visitor /*, Singer::visitor*/ {
        virtual void visit( Athlete* ) override 
        { std::cout << "Athlete puts on the running shoes.\n" ; }
    };
    
    // Athlete has no need for Microphone, so we don't implement Singer::visitor 
    struct Microphone : Person::visitor, /* Athlete::visitor, */ Singer::visitor {
    	virtual void visit( Singer* ) override 
        { std::cout << "Singer sings with the microphone.\n" ; }
    };
    
    // SingingAthlete uses KaraokeWalkman, so we implement SingingAthlete::visitor 
    struct KaraokeWalkman : Person::visitor, SingingAthlete::visitor {
        virtual void visit( SingingAthlete* ) override 
        { std::cout << "SingingAthlete jogs with the KaraokeWalkman.\n" ; }
    };
    
     // ...

}

http://coliru.stacked-crooked.com/a/f59b56dc802175ae
Ok, the uwanted lines are removed, but some wanted lines are removed now.
1
2
3
4
5
6
7
8
9
10
11
Athlete puts on the running shoes.
Athlete has no need for the microphone.  // Don't want this removed!

Singer has no need for the running shoes.  // Don't want this removed!
Singer sings with the microphone.

Athlete puts on the running shoes.
Singer has no need for the running shoes.  // Now removed!  Great!
Athlete has no need for the microphone.  // Now removed!  Great!
Singer sings with the microphone.
SingingAthlete jogs with the KaraokeWalkman. 

Singer WILL take the running shoes in my program (she will simply sell them), and Athlete WILL take the microphone in my program (to sell it). Sorry I didn't mention that.
I'm still studying this new pattern to figure out how to modify it. Also, not a problem yet but it might be later: The class
1
2
3
4
struct KaraokeWalkman : Person::visitor, SingingAthlete::visitor {
        virtual void visit( SingingAthlete* ) override 
       { std::cout << "SingingAthlete jogs with the KaraokeWalkman.\n" ; }
    };

will be problematic later on when I introduce classes like KaraokeWalkman (no such class exists yet, but probably later on will), because I didn't mention that I will be creating (arbitrarily) many classes that are derived from both Singer and Athlete (SingingAthlete is the first of 4 so far). I know that variadic templates covers inheritance too, so I'm sure that problem will be fixed easily by myself when I cross that bridge. The currently manual solution of course is:
1
2
3
4
5
6
7
8
9
10
11
struct KaraokeWalkman : Person::visitor, SingingAthlete::visitor, ChantingRunner::visitor, 
            CarollingSwimmer::vistor, OperaticSkater::visitor {
        virtual void visit( SingingAthlete* ) override 
    { std::cout << "Takes the KaraokeWalkman and uses it.\n" ; }
        virtual void visit( ChantingRunner* ) override 
    { std::cout << "Takes the KaraokeWalkman and uses it.\n" ; }  // same output
        virtual void visit( CarollingSwimmer* ) override 
    { std::cout << "Takes the KaraokeWalkman and uses it.\n" ; } // same output
        virtual void visit( OperaticSkater* ) override 
    { std::cout << "Takes the KaraokeWalkman and uses it.\n" ; } // same output
};  // BLAH! 
Last edited on
I apologize. :( I just saw AtheleteSinger and how Athelete and Singer inherited from the same parent and assumed it was a diamond inheritance issue.
@Daleth: Not at all. When you said "you'll still probably need to be explicit", you prompted me to come up with a different visitor pattern that gives the exact output I want. Though the solution is not ideal and causes some damage to my current program, at least I have something to fall back on if JLBorges' ideal acyclic visitor pattern idea does not work out for my specs. It almost does so far, but not quite yet. I still need to study it more to figure out how to modify it to work perfectly.

I think its just a matter of putting more visit overloads into RunningShoes, Microphone, etc.... But then that runs into the new problem I mentioned above that there shall be SingingAthlete, ChantingRunner, CarollingSwimmer, OperaticSkater, and arbitrarily more classes derived from both Singer and Athlete.
Last edited on
Ok, I solved the problem using JLBorge's acyclic method:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
#include <iostream>
#include <string>
#include <list>

struct LivingBeing { 
    struct ScrollVisitor { virtual ~ScrollVisitor() = default ; };
    virtual ~LivingBeing() = default ;
	virtual void takesItem( ScrollVisitor* ) = 0;
	virtual std::string getName() const = 0;
};

struct MagicUserSpellCastingMonster : public virtual LivingBeing {
    struct ScrollVisitor {
        virtual ~ScrollVisitor() = default ;
        virtual void visit( MagicUserSpellCastingMonster* ) = 0 ;
   };
	virtual void takesItem( LivingBeing::ScrollVisitor* scroll ) override {
        auto s = dynamic_cast<MagicUserSpellCastingMonster::ScrollVisitor*>(scroll) ;
        if(s) s->visit(this) ; 
    }
    virtual std::string getName() const override {return "MagicUserSpellCastingMonster";}
};

struct ClericSpellCastingMonster : public virtual LivingBeing {
    struct ScrollVisitor {
        virtual ~ScrollVisitor() = default ;
        virtual void visit( ClericSpellCastingMonster* ) = 0 ;
   };
    virtual void takesItem( LivingBeing::ScrollVisitor* scroll ) override {
        auto s = dynamic_cast<ClericSpellCastingMonster::ScrollVisitor*>(scroll) ;
        if(s) s->visit(this) ; 
    }
    virtual std::string getName() const override {return "ClericSpellCastingMonster";}
};

struct GoblinMageCleric : public MagicUserSpellCastingMonster, public ClericSpellCastingMonster {
    struct ScrollVisitor {
        virtual ~ScrollVisitor() = default ;
        virtual void visit( GoblinMageCleric* ) = 0 ;
    };
    virtual void takesItem( LivingBeing::ScrollVisitor* scroll ) override {
        auto s = dynamic_cast<GoblinMageCleric::ScrollVisitor*>(scroll) ;
        
        if(s) s->visit(this) ;         
        else 
        {
            MagicUserSpellCastingMonster::takesItem(scroll) ;
            ClericSpellCastingMonster::takesItem(scroll) ;
        }
    }
    virtual std::string getName() const override {return "GoblinMageCleric";}
};

struct InventoryItem {};
struct MagicalItem: public InventoryItem {};
struct Scroll: public MagicalItem {};

struct MagicUserScroll:  public Scroll,  public LivingBeing::ScrollVisitor,  
public MagicUserSpellCastingMonster::ScrollVisitor,  public ClericSpellCastingMonster::ScrollVisitor,
		public GoblinMageCleric::ScrollVisitor {
    virtual void visit (MagicUserSpellCastingMonster*) override 
{std::cout << "Takes the MagicUserScroll.\n";}
	virtual void visit (ClericSpellCastingMonster*) override 
{std::cout << "Takes the MagicUserScroll but cannot use it.\n";}
	virtual void visit (GoblinMageCleric*) override 
{std::cout << "Takes the MagicUserScroll.\n";}
};

struct ClericScroll:  public Scroll,  public LivingBeing::ScrollVisitor, 
public MagicUserSpellCastingMonster::ScrollVisitor,  public ClericSpellCastingMonster::ScrollVisitor,
		public GoblinMageCleric::ScrollVisitor {
	virtual void visit (MagicUserSpellCastingMonster*) override 
{std::cout << "Takes the ClericScroll but cannot use it.\n";}
	virtual void visit (ClericSpellCastingMonster*) override 
{std::cout << "Takes the ClericScroll.\n";}
	virtual void visit (GoblinMageCleric*) override 
{std::cout << "Takes the ClericScroll.\n";}
};

struct MagicUserClericScroll : LivingBeing::ScrollVisitor, GoblinMageCleric::ScrollVisitor {
    virtual void visit (GoblinMageCleric*) override {std::cout << "Takes the MagicUserClericScroll.\n";}
};

int main() {
    std::list<LivingBeing*> beings = { new MagicUserSpellCastingMonster, new ClericSpellCastingMonster, new GoblinMageCleric };
    LivingBeing::ScrollVisitor* magicalItems[] = { new MagicUserScroll,  new ClericScroll, new MagicUserClericScroll };
 
    for( LivingBeing* x: beings )
    {
    	std::cout << x->getName() << ":" << std::endl;
        for( auto item : magicalItems ) x->takesItem(item) ;
        std::cout << '\n' ;
    }
    std::cin.get();
} 

Output:
1
2
3
4
5
6
7
8
9
10
11
12
MagicUserSpellCastingMonster:
Takes the MagicUserScroll.
Takes the ClericScroll but cannot use it.

ClericSpellCastingMonster:
Takes the MagicUserScroll but cannot use it.
Takes the ClericScroll.

GoblinMageCleric:
Takes the MagicUserScroll.
Takes the ClericScroll.
Takes the MagicUserClericScroll.

But here is the NEW problem. There will be MANY derived classes just like GoblinMageCleric:
struct OrcMageCleric : public MagicUserSpellCastingMonster, public ClericSpellCastingMonster {...};
struct OgreMageCleric : public MagicUserSpellCastingMonster, public ClericSpellCastingMonster {...};
struct SkeletonMageCleric : public MagicUserSpellCastingMonster, public ClericSpellCastingMonster {...};
etc...

So until I've resolved this new mess, I cannot yet decide whether to use the cyclic or acyclic visitor pattern. Perhaps I should just tidy up the mess using a macro:
1
2
3
4
5
#define ALL_PARENTS
public LivingBeing::ScrollVisitor,  public MagicUserSpellCastingMonster::ScrollVisitor,  
public ClericSpellCastingMonster::ScrollVisitor, public GoblinMageCleric::ScrollVisitor,
 public OrcMageCleric::ScrollVisitor, OgreMageCleric::ScrollVisitor, 
SkeletonMageCleric::ScrollVisitor, ....

and simply update the macros every time I define a new such derived class. Similarly, use macros for the common bodies of ALLLLLL the identical visit overrides for each new derived class.
Last edited on
> Perhaps I should just tidy up the mess using a macro:
> and simply update the macros every time I define a new such derived class.
> Similarly, use macros for the common bodies of ALLLLLL the identical visit overrides

I would suggest something along these lines, instead of macros:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
template < typename... TYPES > struct base_classes {} ;

template < typename FIRST, typename... REST > 
struct base_classes< FIRST, REST...> : public FIRST, public base_classes<REST...> {} ;

template <> struct base_classes<> {} ;

struct A { virtual ~A() = default ; }; 
struct B { virtual ~B() = default ; };
struct C { virtual ~C() = default ; }; 
struct D {  virtual ~D() = default ; };

struct all_base_classes : public base_classes<A,B,C,D> {};
struct only_a_and_c : public base_classes<A,C> {} ;

struct X : public all_base_classes {} ;
struct Y : public only_a_and_c {} ;
struct Z : private base_classes<A,B,D> {} ;
Last edited on
Topic archived. No new replies allowed.