3

A little background may be needed, but skip to Problem if you feel confident. Hopefully the summary gets the point across.

Summary

I have an InputDispatcher which dispatches events (mouse, keyboard, etc...) to a Game object.

I want to scale InputDispatcher independently of Game: InputDispatcher should be able to support more events types, but Game should not be forced to use all of them.

Background

This project uses JSFML.

Input events are handled through the Window class via pollEvents() : List<Event>. You must do the dispatching yourself.

I created a GameInputDispatcher class to decouple event handling from things such as handling the window's frame.

Game game = ...;
GameInputDispatcher inputDispatcher = new GameInputDispatcher(game);

GameWindow window = new GameWindow(game);

//loop....
inputDispatcher.dispatch(window::pollEvents, window::close);
game.update();
window.render();

The loop has been simplified for this example

class GameInputDispatcher {
    private Game game;

    public GameInputDispatcher(Game game) {
        this.game = game;
    }

    public void dispatch(List<Event> events, Runnable onClose) {
        events.forEach(event -> {
            switch(event.type) {
                case CLOSE: //Event.Type.CLOSE
                    onClose.run();
                    break;
                default:
                    // !! where I want to dispatch events to Game !!
                    break;
            }
        }
    }
}

The Problem

In the code directly above (GameInputDispatcher), I could dispatch events to Game by creating Game#onEvent(Event) and calling game.onEvent(event) in the default case.

But that would force Game to write the implementation for sorting & dispatching mouse and keyboard events:

class DemoGame implements Game {
    public void onEvent(Event event) {
        // what kind of event?
    }
}

Question

If I wanted to feed events from InputDispacher into Game, how could I do so while avoiding Interface Segregation Principle violations? (by declaring all listening methods: onKeyPressed,onMouseMoved, etc.. inside ofGame`, even though they may not be used).

Game should be able to choose the form of input it wants to use. The supported input types (such as mouse, key, joystick, ...) should be scaled through InputDispatcher, but Game should not be forced to support all these inputs.

My Attempt

I created:

interface InputListener {
    void registerUsing(ListenerRegistrar registrar);
}

Game would extend this interface, allowing InputDispatcher to depend on InputListener and call the registerUsing method:

interface Game extends InputListener { }

class InputDispatcher {
    private MouseListener mouseListener;
    private KeyListener keyListener;

    public InputDispatcher(InputListener listener) {
        ListenerRegistrar registrar = new ListenerRegistrar();
        listener.registerUsing(registrar);

        mouseListener = registrar.getMouseListener();
        keyListener = registrar.getKeyListener();
    }

    public void dispatch(List<Event> events, Runnable onClose) {
        events.forEach(event -> {
            switch(event.type) {
                case CLOSE:
                    onClose.run();
                    break;
                case KEY_PRESSED:
                     keyListener.onKeyPressed(event.asKeyEvent().key);
                     break;
                 //...
            }
        });
    }
}

Game subtypes can now implement whatever listener is supported, then register itself:

class DemoGame implements Game, MouseListener {
   public void onKeyPressed(Keyboard.Key key) {

   }

    public void registerUsing(ListenerRegistrar registrar) {
        registrar.registerKeyListener(this);
        //...
    }
}

Attempt Issues

Although this allows Game subtypes to only implement the behaviors they want, it forces any Game to declare registerUsing, even if they don't implement any listeners.

This could be fixed by making registerUsing a default method, having all listeners extend InputListener to redeclare the method:

interface InputListener {
    default void registerUsing(ListenerRegistrar registrar) { }
}

interface MouseListener extends InputListener {
    void registerUsing(ListenerRegistrar registrar);

    //...listening methods
}

But this would be quite tedious to do for every listener I choose to create, violating DRY.

Vilius K.
  • 5
  • 3
Vince
  • 14,470
  • 7
  • 39
  • 84

2 Answers2

1

I do not see any point in registerUsing(ListenerRegistrar). If code external to the listener must be written which knows that this is a listener and therefore it needs to register with a ListenerRegistrar, then it may as well go ahead and register the listener with the registrar.

The Problem as stated in your question is usually handled in GUIs is by means of default processing, using either inheritance or delegation.

With inheritance, you would have a base class, call it DefaultEventListener or BaseEventListener, whatever you prefer, which has a public void onEvent(Event event) method that contains a switch statement which checks the type of event and invokes an overridable on itself for every event that it knows about. These overridables generally do nothing. Then your "game" derives from this DefaultEventListener and provides overriding implementations only for the events that it cares about.

With delegation you have a switch statement in which you check for the events that you know about, and in the default clause of your switch you delegate to some defaultEventListener of type DefaultEventListener which probably does nothing.

There is a variation which combines both: event listeners return true if they process the event, in which case the switch statement immediately returns so that the event will not be processed any further, or false if they do not process the event, in which case the switch statement breaks, so code at the end of the switch statement takes control, and what it does is to forward the event to some other listener.

An alternative approach (used in many cases in SWT for example) involves registering an observer method for each individual event that you can observe. If you do this then be sure to remember to deregister every event when your game object dies, or else it will become a zombie. Applications written for SWT are full of memory leaks caused by gui controls that are never garbage-collected because they have some observer registered somewhere, even though they are long closed and forgotten. This is also a source of bugs, because such zombie controls keep receiving events (for example, keyboard events) and keep trying to do things in response, even though they have no gui anymore.

Mike Nakis
  • 56,297
  • 11
  • 110
  • 142
  • `registerUsing` was a way to avoid `Game` knowing about it's containers (which `InputDispatcher` is one of them). `InputListenerRegistrar` decouples the registering from `InputDispatcher`. This allows me to remove replace the `InputDispatcher` with whatever I'd like, encouraging reusability. – Vince Feb 03 '17 at 23:22
  • As for `DefaultEventListener`, I do not want to perform any more type checking than I already do (which I only do cause JSFML basically forces it). Using that technique still violates ISP, as methods that aren't used will still be declared. Even though `DemoGame` doesn't have to declare it, it's still in `DemoGame`'s interface. "Do nothing" methods themselves are code smells. I could have kept `registerUsing` default and not redeclare it in `KeyListener` and `MouseListener` if I wanted that kind of behavior. – Vince Feb 03 '17 at 23:28
  • That's how several existing guis do it. (Win32, MFC, WinForms, Swing, SWT, a gui that I have built myself in its entirety, etc.) So, like it or not, smelly or not, that's how a lot of people who have a lot of experience on these matters have decided to do it. And you know how everyone thinks they know better and they will do it differently? Well, in this case, they generally don't do it differently. There must be a reason for it. Do what you will with this information. – Mike Nakis Feb 04 '17 at 15:36
  • "*Then your `Game` derives from this DefaultEventListener and provides overriding implementations only for the events that it cares about.*" - Not a single GUI toolkit does this/encourages this. I don't mean to sound jumpy, but it's bad design, flooding the type's interface (which is a huge no-no). Next thing you know [I'm throwing `UnsupportedOperationExceptions`](http://softwareengineering.stackexchange.com/questions/337850/is-expecting-the-api-user-to-implement-an-unsupportedoperationexception-okay). Mind giving me an example of where you've seen this? – Vince Feb 04 '17 at 17:16
  • Look at this: https://msdn.microsoft.com/en-us/library/system.windows.forms.control(v=vs.110).aspx See all the overridable methods that begin with "On...()"? They do nothing. You override them if you need to do something. – Mike Nakis Feb 04 '17 at 17:21
  • Look how massively large that interface is. That's exactly what I'm trying to avoid. Could you give me a non-C++ example? Considering my question is about Java, the two languages have two different sets of devs, and it's no secret that Java devs tend to be a bit nearer about how they write code. And I really hate sounding bias like that, don't get me wrong, but the verbosity of C++ tends to have an affect on design mentality. – Vince Feb 04 '17 at 17:32
  • That's not just C++, that's dotNet, so on the page you saw there is a tab that you can use to select C#, C++, F#, VB. Specifically C# is very much like java. There is no point in trying to avoid a massive interface, because only one base framework class needs to be so large. I repeat: you override only what you want to use. – Mike Nakis Feb 04 '17 at 17:56
  • What I'm saying is I can achieve the same behavior you are suggesting by making `registerUsing` a `default` method. Doing so would only expose 1 "do nothing" method as opposed to a scalable amount of "do nothing" methods (what if I wanted to support a new peripheral, such as VR? That would require a few more methods, which may not be used). That 1 "do nothing" is still a code smell, which suggests there's a deeper design issue, which I'm trying to pinpoint. – Vince Feb 04 '17 at 18:51
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/134866/discussion-between-vince-emigh-and-mike-nakis). – Vince Feb 04 '17 at 19:42
0

While reiterating the issue to a friend, I believe I found the issue.

Although this allows Game subtypes to only implement the behaviors they want, it forces any Game to declare registerUsing, even if they don't implement any listeners.

This suggests Game is already violating ISP: if clients won't use listeners, Game should not derive from InputListener.

If, for some reason, a Game subtype did not want to use listeners (maybe interaction is handled via Web pages or the local machine), Game should not be forced to declare registerUsing.

Solution

Instead, an InteractiveGame could derive from Game and implement InputListener:

interface Game { }
interface InteractiveGame extends Game, InputListener { }

The framework would then have to check the type of Game to see if it needs to instantiate an InputDispatcher:

Game game = ...;
if(game instanceof InteractiveGame) {
    // instantiate input module
}

If someone can suggest a better design, please do so. This design was an attempt to decouple event dispatching from programs that want to make use of user events, while enforcing strong compile-time-safety.

Community
  • 1
  • 1
Vince
  • 14,470
  • 7
  • 39
  • 84