2

I was improving my visitor pattern code in c++ which originally looked like the code right below, in order to get rid of the unnecessary part where the visitor needs to pass itself to the accept void.

I came up with a simple solution which "turns" the parameter into a static field so I can initialise the field right before I ever use accept.

Forgive me but I need to do this even if it will be very ugly.

Old Code:

class A_Child;
class B_Child;

class Father {
    public:
    virtual ~Father() = default;
    class Visitor {
        public:
        virtual void visitChildA(A_Child * child) = 0;
        virtual void visitChildB(B_Child * child) = 0;
    };
    virtual void accept(Visitor * visitor) = 0;
};

class A_Child: public Father {
    public: /* A Child Stuff */
    void accept(Visitor * visitor) override;
};

class B_Child: public Father {
    public: /* B Child Stuff */
    void accept(Visitor * visitor) override;
};

A_Child::accept(Visitor * visitor) { visitor -> visitChildA(this); }
B_Child::accept(Visitor * visitor) { visitor -> visitChildB(this); }

New Code:

class Father {
    public:
    virtual ~Father() = default;
    class Visitor {
        public:
        virtual void visitChildA(A_Child * child) = 0;
        virtual void visitChildB(B_Child * child) = 0;
    };
    static Visitor * visitor;
    virtual void accept() = 0;
};

// Outside, (assuming NewVisitor: public Father::Visitor):

void NewVisitor::DoStuff(Father * child) {
    Father::visitor = this;
    child -> accept();
}

Linker Complaints:

My linker is not happy about it and it gives me the following output:

Undefined symbols for architecture x86_64:
  "Father::visitor", referenced from:
      Child_A::accept() in Father.o
      Child_B::accept() in Father.o
ld: symbol(s) not found for architecture x86_64

Does anyone know why is this happening? Am I breaking some mysterious OOP rule?

Cristian
  • 654
  • 1
  • 13
  • 30
  • Where and how do you initialize `Father::visitor`? You have to define it. – MSpiller Feb 17 '20 at 11:45
  • `static Visitor * visitor;` only *declares* the `visitor` variable, you need to *define* it as well. – Some programmer dude Feb 17 '20 at 11:46
  • I create a new visitor and, right before calling accept, `Father::visitor = this;` – Cristian Feb 17 '20 at 11:47
  • I edited the code to include the part where it accepts the visitor – Cristian Feb 17 '20 at 11:50
  • 2
    You have to define the variable some where. A line like `Visitor* Father::visitor = nullptr` in your .cpp file at global scope should do. – MSpiller Feb 17 '20 at 11:53
  • 2
    Is it really a good idea to replace function arguments with static members? – Maxim Egorushkin Feb 17 '20 at 11:57
  • @MaximEgorushkin I know but actually I have more than 100 subclasses and I call accept a lot so I'm forced for performance reasons to optimise that... – Cristian Feb 17 '20 at 11:59
  • 2
    @Cristian Performance wise, a function argument is going to be loaded into a register, whereas accessing that static hits or misses another cache line. Since you cannot link it you haven't benchmarked it yet... – Maxim Egorushkin Feb 17 '20 at 12:00
  • @M.Spiller It worked, you can post it as an answer... – Cristian Feb 17 '20 at 12:01
  • @MaximEgorushkin I'm benchmarking everything so if really is slower I'll revert the changes to the original code – Cristian Feb 17 '20 at 12:02
  • @MaximEgorushkin I benchmarked both solutions for more than 10 minutes and on average they're the same. I'll revert to the original code. – Cristian Feb 17 '20 at 12:16
  • Does this answer your question? [What is an undefined reference/unresolved external symbol error and how do I fix it?](https://stackoverflow.com/questions/12573816/what-is-an-undefined-reference-unresolved-external-symbol-error-and-how-do-i-fix) – Alan Birtles Feb 17 '20 at 12:17
  • @AlanBirtles I don't know, that post is quite difficult to understand, at least I'm not seeing something familiar to my specific case but since it has a ton of answers, how can I sure about it? And also, it should be closed since It doesn't even talk about one specific thing, but rather 10000 things all put together... – Cristian Feb 17 '20 at 12:25

1 Answers1

2

You have to define the variable some where. A line like

Visitor* Father::visitor = nullptr;

in your .cpp file at global scope should do.

MSpiller
  • 3,500
  • 2
  • 12
  • 24