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

Adding a compose key to Windows

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

Problem

I made a piece of software that basically adds the functionality of a compose key to Windows, after switching from one keyboard layout to another (for programming reasons), and being unable to find a piece of proper software that did what I wanted.

This was my first venture into making applications for Windows, never touching languages like C# before. I'm using two libraries (which I assume is written in C and compiled) for hooking and simulating keypresses, as I could not find any way of doing this in C#.

Now, this is bound to have some mistakes, bad practices or potential issues, and without knowing someone that could possibly review my code, I came here. The program is running and I've tested this on both Windows 8.x and Windows 7 and seem to run just fine on both in my tests. I used VS13 community edition, which has been of great help getting started.

The project in its entirety is hosted on GitHub, although I will post the main file here, I'd love to get a review of the entire structure/codebase.

```
using Microsoft.Win32;
using MouseKeyboardActivityMonitor;
using MouseKeyboardActivityMonitor.WinApi;
using System;
using System.Collections.Generic;
using System.Reflection;
using System.Windows.Forms;
using WindowsInput;

namespace Compose
{
public class App : Form
{
[STAThread]
public static void Main()
{
Application.EnableVisualStyles();
Application.SetCompatibleTextRenderingDefault(false);
Application.Run(new App());
}

private KeyboardHookListener keyboardHook;
private InputSimulator simulator;

public int composeIndex = 0;
public String composeString = "";
public bool isComposing;

public Dictionary lowerCase;
public Dictionary upperCase;
public Dictionary combinations;

private NotifyIcon trayIcon;
private ContextMenu trayMenu;

public App()
{
AppDomain.CurrentDomain.Assem

Solution

I'm not vehemently opposed to the use of Regions, but I am against using them inside of methods. I consider it a definitive code smell. Regions inside of methods should be extracted into their own method and called. For example

private void InititalizeComponent()
{
    lowerCase = GetLowerCaseDictionary();
    upperCase = GetUpperCaseDictionary();
    combinations = GetCombinationsDictionary();

    SetupKeyboardHook();

    simulator = new InputSimulator();

    SetupTrayMenu();
    SetupTrayIcon();

}


Now, if a change needs to be made to how any of these things gets setup, you can go directly to that method instead of searching through hundreds of lines of code for the part of the setup you're looking for.

Okay... Let me rephrase this. Extract Methods. Extract Methods everywhere. Your keyboardHook_KeyDown method is 59 lines. I have to scroll several times to see the whole method. We're human, we can't hold that much in our heads at once. Extracting methods isn't just to remove repetition from our code. It's for making methods singly responsible and small enough to understand at a glance. If we want the detail, we dive into the smaller methods. Otherwise, we get a quick overview of what's happening without concerning ourselves with the details.

Your Settings class is much better in this regard, but there are other issues with it.

  • All of the methods are static, so you might as well make the class itself static. There's no sense in ever having instances of it, there is no state.



  • It's arguably preferable to use an XML configuration over the Windows Registry. Please see What is App.config in C#.Net and How do I use it? However, there's nothing wrong with using the registry. It can be considered to be a matter of preference.



-
If you decide to stick with the registry, you should change where you're registering your keys to.

private static String appPath = "HKEY_CURRENT_USER\\Software\\Compose";
private static String runPath = "HKEY_CURRENT_USER\\Software\\Microsoft\\Windows\\CurrentVersion\\Run";


Take a look at the other keys under Software. Note how all (most) of them are in the form of Company\NameOfSoftware\. You should follow this convention. It makes it infinitely less likely that you'll run into a "namespace" conflict with some other piece of software. Since this is open source software, I recommend using your github user name.

Code Snippets

private void InititalizeComponent()
{
    lowerCase = GetLowerCaseDictionary();
    upperCase = GetUpperCaseDictionary();
    combinations = GetCombinationsDictionary();

    SetupKeyboardHook();

    simulator = new InputSimulator();

    SetupTrayMenu();
    SetupTrayIcon();

}
private static String appPath = "HKEY_CURRENT_USER\\Software\\Compose";
private static String runPath = "HKEY_CURRENT_USER\\Software\\Microsoft\\Windows\\CurrentVersion\\Run";

Context

StackExchange Code Review Q#74873, answer score: 2

Revisions (0)

No revisions yet.