views:

189

answers:

2

My application is developped in C++ using Qt and is using signals and slots.

Let's say I have the following classes (pseudo-C++ code) :

class Ball
{
    Color m_Color;
    int m_Size;
};

class Player
{
public:
    setBall(Ball* pBall)
    {
        if (pBall != m_pBall)
        {
            Ball* pPreviousBall = m_pBall;
            m_pBall = pBall;
            emit notifyBallNotUsed(pPreviousBall);
        }
    }

    Ball* getBall();

signals:
    void notifyBallNotUsed(Ball*);

private:
    String m_Name;
    Ball* m_pBall;
};

class GeneralHandler
{
public:
    addBall(Ball* pBall);
    deleteBall(Ball* pBall);


    addPlayer(Player* pPlayer)
    {
        connect(pPlayer, SIGNAL(notifyBallNotUsed(Ball*)), this, SLOT(onBallUsageChanged(Ball*)));
        ...
    }
    deletePlayer(Player* pPlayer);
    {
        disconnect(pPlayer, SIGNAL(notifyBallNotUsed(Ball*)), this, SLOT(onBallUsageChanged(Ball*)));

        onBallUsageChanged(pPlayer->getBall());
        ....
    }

private slots:
    void onBallUsageChanged(Ball* pBall)
    {
        if (isNotUsedAnymore(pBall))
        {
            m_BallList.remove(pBall);
            delete pBall;
        }
    }

private:
    bool isNotUsedAnymore(Ball* pBall); // Check if the given ball is still used by at least one player

    List<Player*> m_PlayerList;
    List<Ball*> m_BallList;
};

With my application, the user can add/remove player, and for each player, decide the color and the size of the ball. Behind the hood, the GeneralHandler is in charge to store the balls and delete them. It is perfectly possible that two players are using the same ball.

When a player is deleted, if the ball is not used anymore, the GeneralHandler should delete it (or keep it if the ball is still used by another player). If the ball a player is using is changed, the previous ball, if not used anymore, should be deleted by the GeneralHandler as well.

So far so good.

Now, I want to add undo/redo capability to my application, using the command pattern, and this is where I'm stuck. Lets say I have something like this :

class ChangePlayerBall : public QUndoCommand
{
public:
    ChangePlayerBall(Player* pPlayer, Ball* pNewBall)
    {
        m_pPlayer = pPlayer;
    }

    void redo();
    void undo();

private:
    Player* m_pPlayer;
};

I guess the redo() method will look like this :

void ChangePlayerBall::redo()
{
    m_pPlayer->setBall(pNewBall);
}

If nothing else is changed in the code above, the previous Ball will be deleted if not used anymore by other players. This will be a problem when implementing the undo() method : if the previous ball has been deleted, I don't know what was it's characteristics and the undo command won't be able to recreate it. Or maybe I should store the previous ball, but how will the undo/redo command know if this previous ball is still existing or has been deleted by the handler ? Or maybe this mechanism of deleting a ball as soon as it isn't used anymore should be implemented in the undo command ? The problem is that the undo command will have a lot of dependencies on many other classes. The other problem is that this code would be partially duplicated in the DeletePlayer command, which will have to do something similar :

class DeletePlayer : public QUndoCommand
{
public:
    DeletePlayer(Player* pPlayer);

    void redo();
    void undo();
...
};

I hope my explainations where understandable !

How would you solve this problem ? I cannot find a satisfying solution.

Thanks !

+1  A: 

Ball will be deleted if not used anymore by other players

As i can see - its the source of your doubts. Certainly undo() command shouldn't recreate an object nor have its own deleting mechanism. How does your GeneralHandler.isNotUsedAnymore() work? If it counts references to balls, than reference to instance of ChangePlayerBall should be also be counted. Therefore it will be required to connect Command object to some GeneralHandler`s slots.

So, i would suggest:

  1. Ball is deleted when it`s not being used by any players and any UndoCommands (may be including color change & etc)
  2. Linkage between ball and player brakes on new ball assignment as you`ve done
  3. Linkage between ball and command brakes on command`s object destructor (when it completely removed from stack)

Hope it helps )

A: 

How about using a reference counting trick for the ball? When a ball is stored in the command, the command can increment the reference count for the ball, hence preventing from being deleted by the handler (or by itself, depending on how you would change the implementation).

Ariya Hidayat