Need Help Overriding Virtual Function

[atrerus]

I'm in the process of abstracting the transport used for queueing in the OSS project ZDB. I've built an inheritance diagram that looks like this:

https://docs.google.com/drawings/d/1C-b5hCN0SwKREtbx0xIN0c37EGWcm-dWSvj8mpudi_w/edit?usp=sharing

Here is the error I'm getting when building:

Undefined symbols for architecture x86_64:
"zdb::Connection::connect(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)", referenced from:
construction vtable for zdb::ConsumerConnection-in-zdb::KafkaConsumerConnection in kafka_connection.o
construction vtable for zdb::ProducerConnection-in-zdb::KafkaProducerConnection in kafka_connection.o

I get other errors of the same variety, for methods that I've declared virtual in an abstract class and am overriding in a concrete class. But I think if we solve this one it will point me to the right answers for the others.

Here's the specific definitions that I think are relevant for this case.

Connection.h defines virtual connect method:

https://github.com/bitmage/zdb/blob/68756e843817bb2b85583de877b6c1cf2956e68d/src/transport/connection.h#L53-L55

KafkaConnection.h defines an override:

https://github.com/bitmage/zdb/blob/68756e843817bb2b85583de877b6c1cf2956e68d/src/transport/kafka/kafka_connection.h#L31-L33

KafkaConnection.cc defines the implementation:

https://github.com/bitmage/zdb/blob/68756e843817bb2b85583de877b6c1cf2956e68d/src/transport/kafka/kafka_connection.cc#L53-L122

Given that the concrete class being instantiated is KafkaConsumerConnection, and that it inherits from KafkaConnection which provides the implementation, I don't understand why the symbol cannot be found during linking. What am I doing wrong?

Thanks!
Brandon

[ne555]

¿have you provided an implementation for `Connection::connect()'?
If you have no intention on doing so, then declare it as pure virtual.

[atrerus]

Thank you, that helped me fix the problem!

C++ won't let you declare constructors as virtual, so the solution was not exactly as you described. But I did two things.

1) Had to call the Connection() constructor. I did so from the ConsumerConnection and ProducerConnection constructors, which are also abstract. Is this a bad practice? Should I instead leave it to the concrete class to call all constructors or is it ok to have a chain like this?

2) Made all the functions that I was having trouble overriding pure virtual. I don't understand why this worked... maybe because since I had declared them as just virtual, C++ assumed I was providing a definition, and then was erroring because it could not find that definition?

Thanks ne555, you put me on the right path.

[goldenchicken]

1) The chain is perfectly fine, however you should keep in mind that the base part always is constructed first. So this won't work:

class Base
{
private:
    int x;
public:
    Base(int z) : x(z)
    {
    }
}

class Derived : public Base
{
private:
    int y;
public:
    Derived() : y(100), Base::Base(y) /*WRONG! Base::Base() will always be executed first, so y still contains garbage. You compiler will throw a warning at you (wReorder if I'm right)*/
    {

    }
}

2) I guess so.

[ne555]

I'm not well versed on virtual inheritance

for (1) read https://isocpp.org/wiki/faq/multiple-inheritance#virtual-inheritance-ctors

ConsumerConnection::ConsumerConnection() = default;
ConsumerConnection::ConsumerConnection(): Connection() {};

those lines should be equivalent.
and because you've got virtual inheritance, I think that that invocation to `Connection()' will be ignored, it will be executed in `KafkaConsumerConnection'.
you seem to alway use the default constructor, you shouldn't need to write anything

by the way, https://isocpp.org/wiki/faq/multiple-inheritance#virtual-inheritance-where
class KafkaConsumerConnection : /*virtual*/ public KafkaConnection, /*virtual*/ public ConsumerConnection {
dunno what difference does it make.

> maybe because since I had declared them as just virtual, C++ assumed I was
> providing a definition
it didn't assume anything, by not declaring them pure virtual you say that you'll provide a definition.

class base{
	public:
		virtual void foo() = 0;
		virtual void bar(); //no definition provided
};
class derived{
	public:
		virtual void foo() override; //provided a definition, so it isn't an abstract class
};

derived asdf; //can be instantiated because it isn't an abstract class
asdf.bar(); //¿what should this do?
base *fdsa = &asdf;
fdsa->bar(); //¿what should this do? 

one last thing, you should declare the destructor virtual
https://isocpp.org/wiki/faq/virtual-functions#virtual-dtors

[coder777]

C++ won't let you declare constructors as virtual

You are correct. Simply omit constructors that do nothing.

destructors however can and should be virtual in a class that serves as a base. Otherwise the chain of destruction may be broken. See:

http://en.cppreference.com/w/cpp/language/destructor

[atrerus]

Thanks for the additional context, goldenchicken, ne555, coder777. It's more clear to me now. :-)