2

I'm trying to use interface classes and I have the following class structure:

IBase.h:

#pragma once
class IBase
{
protected:
    virtual ~IBase() = default;

public:
    virtual void Delete() = 0;

    IBase& operator=(const IBase&) = delete;
};

IQuackable.h:

#ifndef IQUACKABLE
#define IQUACKABLE

#include "IBase.h"
#include <iostream>


class IQuackable : public IBase
{
protected:
    IQuackable() = default;
    ~IQuackable() = default;

public:
    virtual void Quack() = 0;

    static IQuackable* CreateInstance();
};

#endif // 

MallardDuck.h:

#pragma once
#include "IQuackable.h"

class MallardDuck : public IQuackable
{
private:
    MallardDuck();

protected:
    ~MallardDuck();

public:
    void Delete() override;
    void Quack() override;

    friend IQuackable* IQuackable::CreateInstance();
};

MallardDuck.cpp:

#include "MallardDuck.h"

MallardDuck::MallardDuck() {}

MallardDuck::~MallardDuck() {}

void MallardDuck::Delete() { delete this; }

void MallardDuck::Quack()
{
    std::cout << "Quack!\n";
}

IQuackable* IQuackable::CreateInstance()
{
    return static_cast<IQuackable*>(new MallardDuck());
}

Also I've created class RedHeadDuck.h and .cpp with the same declaration and definition as MallardDuck.

And, finaly, main class code:

#include "MallardDuck.h"
#include "RedHeadDuck.h"

int main()
{
    IQuackable* mallardDuck = MallardDuck::CreateInstance();
    IQuackable* redHeadDuck = RedHeadDuck::CreateInstance();

    mallardDuck->Quack();
    redHeadDuck->Quack();
}

And here I got two errors:


LNK2005 "public: static class IQuackable * __cdecl IQuackable::CreateInstance(void)" (?CreateInstance@IQuackable@@SAPAV1@XZ) already defined in MallardDuck.obj".

LNK1169 "one or more multiply defined symbols found".


As I find out, the problem is in double definition, but how it fix?

I've read about Header guards, but, as I understood, it can't help in this case. Also people write about inline functions, but I've not realized how it may be used here.

What can I do?

Lajos Arpad
  • 64,414
  • 37
  • 100
  • 175
NordMan
  • 127
  • 1
  • 6
  • 3
    `{ delete this; }`... ouch. – Jarod42 Jan 24 '22 at 16:36
  • Remove `IQuackable::CreateInstance()` and create `MallardDuck::CreateInstance()` and `RedHeadDuck::CreateInstance()`? You cannot have two definitions for single function, even if they are in different files. `static` member functions or not, it will not work. – Yksisarvinen Jan 24 '22 at 16:38
  • `MallardDuck::CreateInstance()` is actually `IQuackable::CreateInstance()`... – Jarod42 Jan 24 '22 at 16:41
  • `RedHeadDuck::CreateInstance();` is probably the same, so the duplicated one. – Jarod42 Jan 24 '22 at 16:42
  • According to your description, there *are* two definitions of `IQuackable::CreateInstance()`, in separate files. What is the point of this function? – molbdnilo Jan 24 '22 at 16:43
  • @Jarod42 maybe it's supposed to be a comment `// TODO delete this`. I know our legacy code base is full of these comments xD – JHBonarius Jan 27 '22 at 10:18
  • @JHBonarius: Not sure what that comment would mean either... Anyway, with smart pointers, no need to write `new`/`delete` :) – Jarod42 Jan 27 '22 at 10:22
  • @Jarod42 In my case it sadly means that prototyping code has made it to production. Written long ago by developers that have long left. Anyhow, offtopic. Deleting `this` is not good, no. The OP needs to read a [good book](https://stackoverflow.com/a/388282) – JHBonarius Jan 27 '22 at 10:26

1 Answers1

0

Goals

I suppose these are what you are trying to obtain by adopting all the complicated patterns:

  1. interface, i.e., "multiple types, same set of methods"
  2. some sort of abstract factory pattern, i.e., you want a "instantiator" who provides a static method (which is not very different from a global function) to call, and returns instances of derived classes
  3. prohibit users from directly calling the ctors of derived classes
  4. take care of the dtors by implementing the Delete() method

Requirement 1-3

There are at least 3 ways to meet requirement 1-3, as explained below:

1. Derived classes hiding static method of their base

This is the easiest way, and it's fully capable of current main.cpp. Derived classes can override static methods of their base class.

In file MallardDuck.h and RedHeadDuck.h:

    // Replace this:
    // friend IQuackable* IQuackable::CreateInstance();

    // With this:
    static IQuackable* CreateInstance();

In file MallardDuck.cpp (and RedHeadDuck.cpp similarly):

// Replace this:
// IQuackable* IQuackable::CreateInstance() {
//     return static_cast<IQuackable*>(new MallardDuck());
// }

// With this:
IQuackable* MallardDuck::CreateInstance() {
    return new MallardDuck();
}

The problem with this is that: other derived classes that don't override and hide CreateInstance() will still expose IQuackable::CreateInstance() as a "fallback". Thus:

  1. if you don't actually implement IQuackable::CreateInstance() (so far, you don't have to), then once it is called via a derived class, the code won't compile and won't give a reason that's comprehensible to others; or
  2. if you choose to implement it, you'd better throw an exception within it, which may surprise your user; otherwise you would have to return a nullptr or something, which is the worst practice in C++ (that's what we do in C since it has no language-level support for error handling; any C++ function that cannot fulfill its job should never return).

Either way is not elegant.

2. Adopt abstract factory pattern

This pattern requires a cooperating "factory class", which is abstract; then whenever you derive a concrete quackable, derive also its factory.

In your case you'll need to sketch out a IQuackableFactory, exposing IQuackableFactory::CreateInstance(), then derive a MallardDuckFactory and a RedHeadDuckFactory.

There are plenty of good examples already, so I won't demonstrate here.

3. Feature injection by using CRTP

There's yet another way of doing things. By CreateInstance() you're actually providing a "Give me an instance of this class!" feature. Typically we use the CRTP (curiously recurring template pattern) to "inject" a certain feature into a class.

First write this file EnableCreateInstance.hpp:

#ifndef _ENABLECREATEINSTANCE_HPP_
#define _ENABLECREATEINSTANCE_HPP_

template <class T>
struct EnableCreateInstance {
    static T* CreateInstance() { return new T(); }
};

#endif //_ENABLECREATEINSTANCE_HPP_

Then in MallardDuck.h:

// Add:
#include "EnableCreateInstance.hpp"

class MallardDuck : public IQuackable, public EnableCreateInstance<MallardDuck> {
private:
    MallardDuck();

    friend class EnableCreateInstance<MallardDuck>;

    ...

In file RedHeadDuck.h do the similar: include header, publicly inherit EnableCreateInstance<RedHeadDuck>, and declare EnableCreateInstance<RedHeadDuck> as friend class.

This provides more flexibility: you're still providing an interface CreateInstance(), but in a less "aggressive" way: derived classes have their freedom to choose whether or not to provide CreateInstance(). If they do, just inherit and (if ctor made private) declare friendship; if not, omit the additional inheritance.

Requirement 4

Well, actually you can use delete this in non-static non-dtor method. But:

  1. You must ensure (which can be difficult) that no more access to the deleted object's data members or virtual functions are made, otherwise it causes undefined behaviors;
  2. You leave your users with dangling pointers to the deleted object.

So, we seldom provide such "deleters" in modern C++. You can get all the benefits it may provide through smart pointers, plus the ability to avoid UBs and so much more.

Oasin
  • 90
  • 2
  • 9