Is this crufty?

Question

I'm writing code like:

class Game
{
    int getMouseX()
    {
        return inputManager.getMouseX() ;
    }
} ;

I remember seeing code like this and hating it. One function passes off to another? What is this "pattern" (or, possibly, anti-pattern) called? I don't like it!

On the other hand, it saves exposing the InputManager to the user of the class... would that be a better choice? Or should Game simply not contain InputManager?

Edit

What about using multiple inheritance instead?

class Game : public InputManager, public Window
{
    // by virtue of inheriting InputManager and Window,
    // Game now has "acquired" the capabilities of
    // InputManager's public functions, without requiring delegate.
} ;

Have I not found a reasonably good use for multiple inheritance??

Answer 1

On the contrary, this seems pretty good.

You could separate the Game from the input by hiding it behind an interface and using an IOC container to load the inputManager dynamically...

Answer 2

To add to what John Weldon stated, here is an example:

interface IInputManager
{
    int GetMouseX();
    int GetMouseY();
    Key GetKey();
}

class InputManager: IInputManager
{
    // blah
}

class Game
{
    public Game(IInputManager inputManager) // inject dependencies with an IoC container
    {
        m_inputManager = inputManager;
    }

    private readonly IInputManager m_inputManager;

    public int MouseX
    {
        get { return m_inputManager.GetMouseX(); }
    }

    public int MouseY
    {
        get { return m_inputManager.GetMouseY(); }
    }

    // etc.
}

You should use an IoC container like Castle Windsor or Ninject to handle wiring up your dependencies, but the pattern is a very sound, solid, best practice kind of pattern that is highly recommended these days.

Answer 3

In answer to your other question (name of the pattern), it's called delegation.

Whether its a "good" thing or not depends entirely on where/how you use it. A good use would be to abstract away dealing directly with other dependencies (so for example centralising and hiding use of the inputManager via your Game). However a bad use would be simply exposing all of the inputManager's properties via a separate class (say your Game).

Your aim should be to have each class have a well defined role within your system and collaborate with other objects to perform its responsibilities. In doing this you want to create layers of abstraction that help reduce the amount of dependence the objects have on each other. So with that in mind, I'd say if you were delegating a lot of properties through, I'd start thinking it was a bit of a code smell (since you're mixing responsibilities of the classes).

As a concrete example, I'd say you should have something more like this:

class Game
{
    Point getPlayerTarget()
    {
        return new Point(inputManager.getMouseX(), inputManager.getMouseY());
    }
}

In other words, the Game is responsible for knowing what the player is targeting, but not for the exact x,y coordinates of where the mouse is. This keeps your code nice & clean and allows you to do things like change underlying dependencies (say the method of input in your example), without having to change masses of code (i.e. your Game class wouldn't need to change its interface).

Of course my example removes the direct delegation, which implies that I think its a bad thing... There will, however, be times where it's a good thing. Extending my example above, if there were other methods on your inputManager that other classes might need exactly as is, then it probably does make sense for the Game to delegate these through directly rather than having higher up dependencies need to sometimes talk to Game and sometimes talk to InputManager. Hope that makes sense.

Answer 4

Given the method in your example is private, I think the property is redundant. Presumably you can call inputManager directly from your class methods (injected or or not).

If getMouseX() was public and it was satisfying an interface or Game objects are expected to provide a public getMouseX() method, then it makes sense.

Ultimately I don't think we have enough context or sample code to add much value here.

Answer 5

I'm not entirely sure what language you're using, but it might be possible to do something like this instead:

class Game
{
    getMouseX = inputManager.getMouseX;
};

So that Game.getMouseX is simply an alias for inputManager.getMouseX

If your language doesn't allow that, you could use function pointers if those are allowed.

Answer 6

I'd say the fact that Game knows about a MouseX/MouseY is a bit odd in the first place.

Instead, have Game tell InputManager to update itself - and InputManager an then tell anyone registered with it about any input events that happened.

Something like this...

public class InputManager : IUpdatable
{
    public void Update() { };
}

public interface IUpdatable
{
    void Update();
}
public class Game
{
    public void AddObject(object component)
    {
        m_objects.Add(component);
    }

    public void Update()
    {
        foreach (object obj in m_objects)
        {
            IUpdatable updatable = obj as IUpdatable;
            if (updatable != null)
            {
                updatable.Update();
            }
        }
    }
}

Your main loop then creates the Game object by adding appropriate components, and runs Update in a loop. This avoids tight coupling of your Game to your InputManager, or any other component for that matter.