Preventing keydown delay

I created a class to prevent the default delay which happens when holding down the key on Windows Forms. It seems like the class is working correctly but I don’t know if I’m using the best practice.

KeyboardController class;

class KeyboardController
{
    public event EventHandler kbEvent;
    private DispatcherTimer timer;
    private HashSet<Key> keyPressed;

    public KeyboardController(Window c, EventHandler function)
    {
        c.KeyDown += WinKeyDown;
        c.KeyUp += WinKeyUp;
        kbEvent = function;
        keyPressed = new HashSet<Key>();

        timer = new DispatcherTimer();
        timer.Tick += new EventHandler(kbTimer_Tick);
        timer.Interval = new TimeSpan(0, 0, 0, 0, 10);
        timer.Start();
    }

    public bool KeyDown(Key key)
    {
        if (keyPressed.Contains(key))
        {
            return true;
        }
        else
        {
            return false;
        }
    }

    private void WinKeyDown(object sender, KeyEventArgs e)
    {
        keyPressed.Add(e.Key);
    }

    private void WinKeyUp(object sender, KeyEventArgs e)
    {
        keyPressed.Remove(e.Key);
    }

    private void kbTimer_Tick(object sender, EventArgs e)
    {
        kbEvent(null, null);
    }
}

MainWindow.xaml.cs;

public MainWindow()
{
    EventHandler eh = new EventHandler(KeyboardFunction);
    kbc = new KeyboardController(this, eh);
}

private void KeyboardFunction(object sender, EventArgs e)
{
    if (kbc.KeyDown(Key.Right))
    {
        // do something.
    }
    else
    {
        // do something.
    }
}

1. The main issue is that the key event handler and the timer tick will run on different threads which means that someone could press a key while the timer tick fires – in which case the HashSet could be modified while you are trying to read it from another thread. You will need to safeguard against that by protecting the access to the HashSet with a lock You can probably get away with it because the DispatcherTimer runs on the UI thread where the KeyDown/KeyUp events are also being raised.

2. The idiomatic way of subscribing to event handler is to let the user subscribe to the event rather than passing the event handler via the constructor. What happens if the user has shorter lifetime than the event provider? In your case it would keep it alive because you can’t unsubscribe.

3. When raising the event you need to check if the vent handler is null – subscriptions to an event are considered optional which means no subscriber might exist. Also you should not pass null as sender or EventArgs. The idiomatic pattern in C# goes like this:

   private void kbTimer_Tick(object sender, EventArgs e)
   {
       var handler = kbEvent;
       if (handler != null)
           handler(this, EventArgs.Empty);
   }
   

4. The KeyDown method can be reduced to 1 line of code:

   return keyPressed.Contains(key);
   

5. The name for the event is a bit non-descriptive. How about keyboardTick?

6. Standard naming convention in C# for public fields/properties/methods is PascalCase (so the event should be KeyboardTick).

In summary the final code could look like this:

class KeyboardController
{
    public event EventHandler KeyboardTick;
    private DispatcherTimer timer;
    private HashSet<Key> pressedKeys;
    private readonly object pressedKeysLock = new object();

    public KeyboardController(Window c)
    {
        c.KeyDown += WinKeyDown;
        c.KeyUp += WinKeyUp;
        pressedKeys = new HashSet<Key>();

        timer = new DispatcherTimer();
        timer.Tick += kbTimer_Tick;
        timer.Interval = new TimeSpan(0, 0, 0, 0, 10);
        timer.Start();
    }

    public bool KeyDown(Key key)
    {
        lock (pressedKeysLock)
        {
            return pressedKeys.Contains(key);
        }
    }

    private void WinKeyDown(object sender, KeyEventArgs e)
    {
        lock (pressedKeysLock)
        {
            keyPressed.Add(e.Key);
        }
    }

    private void WinKeyUp(object sender, KeyEventArgs e)
    {
        lock (pressedKeysLock)
        {
            keyPressed.Remove(e.Key);
        }
    }

    private void kbTimer_Tick(object sender, EventArgs e)
    {
        var handler = keyboardTick;
        if (handler != null)
        {
            handler(this, EventArgs.Empty);
        }
    }
}

and the usage:

public MainWindow()
{
    kbc = new KeyboardController(this);
    kbc.KeyboardTick += HandleKeyboardTick;
}

private void HandleKeyboardTick(object sender, EventArgs e)
{
    if (kbc.KeyDown(Key.Right))
    {
        // do something.
    }
    else
    {
        // do something.
    }
}