HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Handling keyboard shortcuts in C# software

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingkeyboardsoftwareshortcuts

Problem

Edit: I have added the revised code in an answer to this question.

Currently, I handle the keyboard shortcuts of my applications in a single huge method that looks like this:

protected override bool ProcessCmdKey(ref Message msg, Keys keyData)
    {
        // Next issue (validate the fix)
        if (keyData == (Keys.Control | Keys.Enter) || 
            keyData == (Keys.Shift | Keys.Enter))
        {
            mark_as_fixed();
            return true;
        }
        // Skip to next issue without validating or changing anything
        if (keyData == (Keys.Alt | Keys.Down))
        {
            next_issue();
        }
        // Previous issue
        if (keyData == (Keys.Alt | Keys.Up))
        {
            previous_issue();
            return true;
        }
        ... [130 lines of this]


It works perfectly, it isn't particularly hard to read or maintain, but having a method 130 lines long and constantly growing just feels wrong.

Also I may want to implement shortcut customization in the future, and there is no easy way to find out what keys are mapped to which method so it will have to be rewritten.

Since all non trivial applications have a gazillion shortcuts, I assume it's a solved problem, but I couldn't find a good explanation on how applications handle their shortcuts.

Solution

Also I may want to implement shortcut customization in the future, and there is no easy way to find out what keys are mapped to which method so it will have to be rewritten.

Stop right here! Extract a class for each command (e.g. instead of a mark_as_fixed method, you'll have a MarkAsFixedCommand class). If you're using WPF you can implement the ICommand interface and have nevermind, you're using WinForms. Still, extract the command classes - WPF's ICommand is a really simple interface that merely exposes bool CanExcute(object) and void Execute(object) methods (you don't have to implement the object parameter if you don't need it) - by extracting your logic into command classes, you can prepare the ground for the day you redo your UI with WPF (which you should!).


there is no easy way to find out what keys are mapped to which method

Make one! Have your command classes derive from an abstract CommandBase class that exposes a method for it:

public abstract bool IsShortcutKey(Keys keys);


Now every derived command class has to implement this IsShortcutKey method. The implementation for MarkAsFixedCommand could look like this:

public override bool IsShortcutKey(Keys keys)
{ 
    return keys == (Keys.Control | Keys.Enter)
        || keys == (Keys.Shift | Keys.Enter); 
}


A cleaner way would have been to expose some Keys ShortcutKey { get; } property getter, but that wouldn't play nice with multiple shortcuts for the same command.

Anyway now that each command implementation is responsible for is shortcut key logic, you can easily fetch the configuration and make that method return true when keys matches whatever Keys value you have in your config.

Then, whoever runs this ProcessCmdKey method simply needs to know about all available commands - take them as an IEnumerable constructor parameter and let whoever is calling this constructor deal with providing it with the command instances (which you will acquire with some reflection code).

Now ProcessCmdKey can be as simple as this:

// assume only 1 command returns true for specified keyData value:
var command = _commands.SingleOrDefault(cmd => cmd.IsShortcutKey(keyData));
if (command != null && command.CanExecute())
{
    command.Execute();
    return true;
}

return base.ProcessCmdKey(ref msg, keyData);


PS - Method names should be PascalCase, not lower_snake_case.

Code Snippets

public abstract bool IsShortcutKey(Keys keys);
public override bool IsShortcutKey(Keys keys)
{ 
    return keys == (Keys.Control | Keys.Enter)
        || keys == (Keys.Shift | Keys.Enter); 
}
// assume only 1 command returns true for specified keyData value:
var command = _commands.SingleOrDefault(cmd => cmd.IsShortcutKey(keyData));
if (command != null && command.CanExecute())
{
    command.Execute();
    return true;
}

return base.ProcessCmdKey(ref msg, keyData);

Context

StackExchange Code Review Q#134893, answer score: 15

Revisions (0)

No revisions yet.