c++ how to know where to forward declare in state pattern

Question

This question comes from this question. Im trying to implement the state pattern with a shared_ptr to the container(game).

However I have a problem with circular inclusion and need to forward declare.

My code:

Game.h

#pragma once
#include <memory>

#include "BaseState.h"
class Game : public std::enable_shared_from_this<Game>
{
private:
    std::shared_ptr<BaseState> currentState;
public:
    Game();
    void switchState(std::shared_ptr<BaseState> nextState);
    void doSomething(char);
    void runState();
};

cpp

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

#include "Game.h"
#include "SomeState.h"

Game::Game()
{
    currentState = std::make_shared<SomeState>();
}

void Game::switchState(std::shared_ptr<BaseState> nextState)
{
    currentState = nextState;
}

void Game::doSomething(char c)
{
    std::cout << "Game : " << c;
}

void Game::runState()
{
    currentState->handleCommand(shared_from_this());
}

BaseState.h

#pragma once
#include <memory>

#include "Game.h"

class BaseState
{
public:
    virtual void handleCommand(std::shared_ptr<Game>) = 0;
};

SomeState.h

#pragma once
#include "BaseState.h"
class SomeState :
    public BaseState
{
public:

    // Inherited via BaseState
    virtual void handleCommand(std::shared_ptr<Game>) override;
};

cpp

#include "stdafx.h"
#include "SomeState.h"

void SomeState::handleCommand(std::shared_ptr<Game> game)
{
    game->doSomething('S');
}

I read other questions about forward declaring but still don't get it.

What I tried;

forward declare BaseState in Game, the code compiles but throws an error.

Unhandled exception at 0x73E9DAE8 in ConsoleApplication1.exe: Microsoft C++ exception: std::bad_weak_ptr at memory location 0x00BBF5D4.

Forward declare Game in BaseState. Dosnt compile gives use of undefined type error, also

'doSomething': is not a member of 'std::shared_ptr'

which is logic because at compile time game has not a doSomething function because forward declared like;

class Game;

How do I decide where to forward declare another class, are there any logical steps or should I just pick one and fix the problems that choise creates?

Forward declare the classes you need in the header files. Then in the cpp files actually include the header file that has the declarations you need. That is the jist of the dupe target. — NathanOliver, Aug 26 '16 at 16:10
The `BaseState.h` file doesn't really need the full `Game` class, just a forward declaration. Same with `Game.h`, it doesn't need a full `BaseState` definition, just a forward declaration. — Some programmer dude, Aug 26 '16 at 16:15
As for the crash you get, it's unrelated to any forward declaration issues you might have. Instead you should use a debugger to locate the crash in your code and try to figure out why it happens. — Some programmer dude, Aug 26 '16 at 16:17
@JoachimPileborg it happens because the OP has `BaseState` taking in a `shared_ptr`, when really it should be taking in a `Game&`, as ownership is not being transfered. The OP's code then attempts to make a `shared_ptr` out of a non-shared object via `shared_from_this()`. — aruisdante, Aug 26 '16 at 16:28
@JoachimPileborg is it safe to say, there is no circular reference if the program compiles? — Sven van den Boogaart, Aug 26 '16 at 16:33

rustyx · Accepted Answer · 2016-08-26T19:17:20.980

You don't need to #include <Game.h> in BaseState.h, you can simply forward-declare it

class Game;

This works because the BaseState declaration doesn't need to know the contents of Game. So what you tried first is OK. The same applies to #include <BaseState.h> in Game.h. Replace that with a forward-declaration of BaseState.

The std::bad_weak_ptr exception was due to something else. Specifically, you're probably missing the little detail about shared_from_this, which says

It is permitted to call shared_from_this only on a previously shared object, i.e. on an object managed by std::shared_ptr. Otherwise the behavior is undefined

and

(from C++17) std::bad_weak_ptr is thrown (by the shared_ptr constructor from a default-constructed weak_this)

You can usually solve this by instantiating your object into a shared_ptr:

int main() {
    auto myGame = std::make_shared<Game>();
    . . .
    myGame->runState();
    . . .
}

EDIT

Keep in mind though, that shared_ptr has a certain cost associated with using it. In general, if you know the pointed-to object always outlives the function call where it is used, as might be the case with your BaseState::handleCommand, then it may be faster (and still safe) to just pass it by reference.

There's still an unneeded circular include. The forward-declare avoids the need to include `BaseState.h` inside the `Game.h` header. Alternatively, the forward-declare could be written to avoid needing the `Game.h` include inside `BaseState.h`. Source files would of course need to be updated as appropriate with the moved includes. — aruisdante, Aug 26 '16 at 16:22
In general, the design of `BaseState` is wrong; `BaseState` does not take ownership of `Game` when it is `doingSomething`. It should just take a standard `Game&` if it needs mutation, or `const Game&` otherwise. This would remove the need for the `shared_from_this()` use in the first place, it could simply call `currentState->doSomething(*this)`. — aruisdante, Aug 26 '16 at 16:25
@RustyX thanks for the answer, how do i know which one to pick BaseState in Game or Game in BaseState? I like to know how I decide which to chose. — Sven van den Boogaart, Aug 26 '16 at 16:43

c++ how to know where to forward declare in state pattern

1 Answers1